Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: ie8e14432:Fastmail
Date:   Sat, 3 Jun 2023 13:25:04 +0900
From:   Takashi Sakamoto <o-takashi@sakamocchi.jp>
To:     Randy Dunlap <rdunlap@infradead.org>
Cc:     linux1394-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org,
        Stephen Rothwell <sfr@canb.auug.org.au>
Subject: Re: [PATCH] firewire: fix warnings to generate UAPI documentation
Message-ID: <20230603042504.GA135015@workstation.local>
Mail-Followup-To: Randy Dunlap <rdunlap@infradead.org>,
        linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        Stephen Rothwell <sfr@canb.auug.org.au>
References: <20230601144937.121179-1-o-takashi@sakamocchi.jp>
 <c9770ddc-d1fe-d49f-adec-a413a7bf65ec@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c9770ddc-d1fe-d49f-adec-a413a7bf65ec@infradead.org>
Precedence: bulk

Hi Randy,

On Thu, Jun 01, 2023 at 01:23:27PM -0700, Randy Dunlap wrote:
> Hi,
> 
> On 6/1/23 07:49, Takashi Sakamoto wrote:
> > Any target to generate UAPI documentation reports warnings to missing
> > annotation for padding member in structures added recently.
> > 
> > This commit suppresses the warnings.
> > 
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Closes: https://lore.kernel.org/lkml/20230531135306.43613a59@canb.auug.org.au/
> > Fixes: 7c22d4a92bb2 ("firewire: cdev: add new event to notify request subaction with time stamp")
> > Fixes: fc2b52cf2e0e ("firewire: cdev: add new event to notify response subaction with time stamp")
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > ---
> >  include/uapi/linux/firewire-cdev.h | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> > 
> 
> You can do it as this patch shows, or you can hide those padding fields as
> described in Documentation/doc-guide/kernel-doc.rst:
> 
> Inside a struct or union description, you can use the ``private:`` and
> ``public:`` comment tags. Structure fields that are inside a ``private:``
> area are not listed in the generated output documentation.
> 
> The ``private:`` and ``public:`` tags must begin immediately following a
> ``/*`` comment marker. They may optionally include comments between the
> ``:`` and the ending ``*/`` marker.
> 
> See below.
> (snip) 

Thanks for your review and suggestion. Indeed, the private/public
annotation is one of our options and I have realized it.

I think it my preference to reduce load when reading structure layout.
The usage of private/public requires readers' brain to switch context
of access modifier. I guess that I would like to avoid such load if I were
the reader.

If the structure definition includes many annotations, it also increases
such load. In my experience, it is likely that annotation in structure
definition does not necessarily include enough information about the
member itself, especially when writing applications. I think it my
preference as well that member annotation of structure is in document,
(but it would be case-by-case.) It seems to be explicit way, vaguely.

Anyway thanks for your comment. It is fun to me;)


Takashi Sakamoto