netdev - Re: [PATCH 3/3] net: phylink: correct trivial kernel-doc inconsistencies

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 22 Jun 2020 13:07:37 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     colton.w.lewis@...tonmail.com
Cc:     andrew@...n.ch, linux@...linux.org.uk, netdev@...r.kernel.org
Subject: Re: [PATCH 3/3] net: phylink: correct trivial kernel-doc
 inconsistencies

From: Colton Lewis <colton.w.lewis@...tonmail.com>
Date: Sun, 21 Jun 2020 23:02:30 +0000

> On Sunday, June 21, 2020 10:53:45 AM CDT Russell King - ARM Linux admin wrote:
>> > ---
>> >   */
>> >  struct phylink_config {
>> >  	struct device *dev;
>> > @@ -331,7 +333,7 @@ void pcs_get_state(struct phylink_config *config,
>> >   *
>> >   * For most 10GBASE-R, there is no advertisement.
>> >   */
>> > -int (*pcs_config)(struct phylink_config *config, unsigned int mode,
>> > +int *pcs_config(struct phylink_config *config, unsigned int mode,
>> >  		  phy_interface_t interface, const unsigned long *advertising);
>> 
>> *Definitely* a NAK on this and two changes below.  You're changing the
>> function signature to be incorrect.  If the documentation can't parse
>> a legitimate C function pointer declaration and allow it to be
>> documented, then that's a problem with the documentation's parsing of
>> C code, rather than a problem with the C code itself.
> 
> I realize this changes the signature, but this declaration is not compiled. It is under an #if 0 with a comment stating it exists for kernel-doc purposes only. The *real* function pointer declaration exists in struct phylink_pcs_ops.
> 
> Given the declaration is there exclusively for documentation, it makes sense to change it so the documentation system can parse it.

I agree with Russell, if the C code can't be accurately represented you
make things worse for people trying to actually _use_ the documentation.

Can't you escape the parenthesis or something like that?

If you can't make it look accurate, leave it alone.