Commit 81929718 authored by Jonathan Corbet's avatar Jonathan Corbet

Merge branch 'dump-struct' into docs-next

André Almeida writes:

The current way that scripts/kernel-doc dump structs do not work for
nested structs with attributes (e.g. __packed) and without type alias
(e.g } alias1;). This is due to the way nested structs members are parsed.

Inside dump_struct(), the regex removes attributes and it surrounds
whitespaces. After that, it will do a foreach(id) loop for each alias.
However, we will have an empty string, and then the split function will
return nothing and the foreach will not have any iteration. The code will
then jump the loop and the substitution will fail since $newmember
is uninitialized.

This bug does not happen when the nested struct has no alias and no
attribute, since at process_proto_type() a whitespace is inserted in
order to ensure that the split will not fail. However, with any
attribute, this whitespace will be removed.

This patch solves this by replacing attributes with one whitespace, to
ensure that will have at least one whitespace.

Besides solving this bug at patch 1, I also add support for the
____cacheline_aligned_in_smp atribute at patch 2.

For testing and reproducing, create a file `code.h`:

/**
 * struct foobar - description
 * @number0: desc0
 * @number1: desc1
 * @number2: desc2
 */
struct foo {
	int number0;

	struct  {
		int number1;
	} __packed;

	struct {
		int number2;
	};

 };

I've tested with CRYPTO_MINALIGN_ATTR, __attribute__((__aligned__(8)))
and __aligned() as well. Now, run the `./script/kernel-doc code.h`,
and this is the output:

Use of uninitialized value $newmember in substitution (s///) at
./scripts/kernel-doc line 1152, <IN> line 18.

.. c:type:: struct foo

   description

**Definition**

::

  struct foo {
    int number0;
    struct {
      int number1;
    };
    struct {
      int number2;
    };
  };

**Members**

``number0``
  desc0

``{unnamed_struct}``
  anonymous

``number2``
  desc2

The output will not display the member number1 and will also display an
uninitialized warning. Running after the patch will get rid of the
warning and also display the description for number1:

[...]

**Members**

``number0``
  desc0

``{unnamed_struct}``
  anonymous

``number1``
  desc1

``{unnamed_struct}``
  anonymous

``number2``
  desc2

To test patch [2/2], just replace __packed for
____cacheline_aligned_in_smp and compare how, at the previous stage the
script believes ____cacheline... is an alias for the struct and after
the patch it is removed just as the others attributes.

Finally, I have compared the output of "make htmldocs" with and without
this patch. No new warning were found and one warning regarding
____cacheline_aligned_in_smp at include/linux/netdevice.h was removed.
parents 2730ce01 f861537d
...@@ -1062,7 +1062,7 @@ sub dump_struct($$) { ...@@ -1062,7 +1062,7 @@ sub dump_struct($$) {
my $x = shift; my $x = shift;
my $file = shift; my $file = shift;
if ($x =~ /(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/) { if ($x =~ /(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|____cacheline_aligned_in_smp|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/) {
my $decl_type = $1; my $decl_type = $1;
$declaration_name = $2; $declaration_name = $2;
my $members = $3; my $members = $3;
...@@ -1073,10 +1073,11 @@ sub dump_struct($$) { ...@@ -1073,10 +1073,11 @@ sub dump_struct($$) {
# strip comments: # strip comments:
$members =~ s/\/\*.*?\*\///gos; $members =~ s/\/\*.*?\*\///gos;
# strip attributes # strip attributes
$members =~ s/\s*__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)//gi; $members =~ s/\s*__attribute__\s*\(\([a-z0-9,_\*\s\(\)]*\)\)/ /gi;
$members =~ s/\s*__aligned\s*\([^;]*\)//gos; $members =~ s/\s*__aligned\s*\([^;]*\)/ /gos;
$members =~ s/\s*__packed\s*//gos; $members =~ s/\s*__packed\s*/ /gos;
$members =~ s/\s*CRYPTO_MINALIGN_ATTR//gos; $members =~ s/\s*CRYPTO_MINALIGN_ATTR/ /gos;
$members =~ s/\s*____cacheline_aligned_in_smp/ /gos;
# replace DECLARE_BITMAP # replace DECLARE_BITMAP
$members =~ s/DECLARE_BITMAP\s*\(([^,)]+),\s*([^,)]+)\)/unsigned long $1\[BITS_TO_LONGS($2)\]/gos; $members =~ s/DECLARE_BITMAP\s*\(([^,)]+),\s*([^,)]+)\)/unsigned long $1\[BITS_TO_LONGS($2)\]/gos;
# replace DECLARE_HASHTABLE # replace DECLARE_HASHTABLE
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment