Re: [PATCH v13 04/19] nfsd: factor out __fh_verify to allow NULL rqstp to be passed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 26, 2024 at 09:44:48AM +1000, NeilBrown wrote:
> On Mon, 26 Aug 2024, Chuck Lever wrote:
> > See comment on 5/N: since that patch makes this a public API again,
> > consider not removing this kdoc comment but rather updating it.
> 
> What exactly do you consider to be a "public API"??  Anything without
> "static"?  That seems somewhat arbitrary.

Anything that is invoked from another, distinct, area of code
presents an API boundary.


> I think of __fh_verify() as a private API used by fh_verify() and
> nfsd_file_acquire_local() and nothing else.

That is indeed the current situation. But practically speaking,
there's nothing that C can do to prevent other call sites from
appearing over time.


> It seems pointless duplication the documentation for __fh_verify() and
> fh_verify().

We have that duplication already for the family of nfsd_file_acquire
APIs, for example.

These two, however, take somewhat different parameters, and
somewhere (in the code) you have to document things like "this one
expects a non-NULL @rqstp". Some API documentation, even for
"private" APIs, is valuable.


> Maybe one could refer to the other "fh_verify is like
> fh_verify except ....."

Highlighting the differences is certainly helpful for human readers.

Perhaps it doesn't have to be a kdoc-style comment, but that's
generally the kind of comment a developer would expect to explain
how and when to invoke a function.


-- 
Chuck Lever




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux