linux-kernel - Re: [PATCH 16/18] mtd: rawnand.h: use nested union kernel-doc markups

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180507083232.5a084ba7@vento.lan>
Date:   Mon, 7 May 2018 08:32:32 -0300
From:   Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
To:     Boris Brezillon <boris.brezillon@...tlin.com>
Cc:     Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...radead.org>,
        linux-kernel@...r.kernel.org, Jonathan Corbet <corbet@....net>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        linux-mtd@...ts.infradead.org
Subject: Re: [PATCH 16/18] mtd: rawnand.h: use nested union kernel-doc
 markups

Hi Boris,

Em Mon, 7 May 2018 11:46:50 +0200
Boris Brezillon <boris.brezillon@...tlin.com> escreveu:

> Hi Mauro,

> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 5dad59b31244..b986f94906df 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -740,8 +740,9 @@ enum nand_data_interface_type {
> >  
> >  /**
> >   * struct nand_data_interface - NAND interface timing
> > - * @type:	type of the timing
> > - * @timings:	The timing, type according to @type
> > + * @type:	 type of the timing
> > + * @timings:	 The timing, type according to @type
> > + * @timings.sdr: Use it when @type is %NAND_SDR_IFACE.  
> 
> Hm, really feels weird to do that. I mean, either we describe
> timings.sdr or timings, but not both. I this case, I agree that
> describing timings.sdr would make more sense than generically
> describing any possible entries in the timings union.

This struct is funny, as the union has just one item. I assume
that the original intend is to be ready to have more timing
types (or you had it in the past). By the time you have a
second value there, describing the union would make more
sense.

> Is there a simple
> way we can get rid of the warning we have when not describing timings
> but all of its fields?

There's no direct way. It won't be hard to add a way to do it,
like applying the enclosed patch with ends by declaring timings as:

	* @timings:	 -- undescribed --

IMHO, this is uglier.

The way I see is that, if the embed struct/union is not interesting
enough to be described, the best would be to use an anonymous one like:

	struct nand_data_interface {
		enum nand_data_interface_type type;
		union {
			struct nand_sdr_timings sdr;
		};
	};

With the above, kernel-doc will expect a description just for @sdr.

Btw, if you want to see how nested struct/union is parsed, the
scripts/kernel_doc logic is at dump_struct() function.

When it finds an struct, it first handles private/public by simply
removing everything that it is not public from the struct, by a
very simple parser. Then, it converts nested struct/union into
un-nested ones. E. g. it converts:

	struct {
		union {
			int foo1;
		};
		union {
			int foo2;
		} bar;
	} foobar;

into this internal representation:

	struct {
		int foo1;
		union;
		int bar.foo2;
		union bar;
	};

Then it runs the normal un-nested struct parser.


Thanks,
Mauro

[PATCH] don't describe nested struct timings

IMHO, this is an ugly hack, but it allows having nested
structs (or fields) undescribed by purpose.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@...nel.org>

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index b986f94906df..b724c7edf532 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -741,7 +741,7 @@ enum nand_data_interface_type {
 /**
  * struct nand_data_interface - NAND interface timing
  * @type:	 type of the timing
- * @timings:	 The timing, type according to @type
+ * @timings:	 -- undescribed --
  * @timings.sdr: Use it when @type is %NAND_SDR_IFACE.
  */
 struct nand_data_interface {
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 0057d8eafcc1..196a2107c8c1 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -390,7 +390,7 @@ my $section = $section_default;
 my $section_context = "Context";
 my $section_return = "Return";
 
-my $undescribed = "-- undescribed --";
+my $undescribed = "-- undescribed --\n";
 
 reset_state();