Re: [RFC] scripts: kernel-doc: fix typedef support for struct parsing
On 23/2/21 3:10 am, Jonathan Corbet wrote: > Aditya Srivastava writes: > >> There are files in kernel, which use 'typedef struct' syntax for defining >> struct. For eg, include/linux/zstd.h, drivers/scsi/megaraid/mega_common.h, >> etc. >> However, kernel-doc still does not support it, causing a parsing error. >> >> For eg, running scripts/kernel-doc -none on include/linux/zstd.h emits: >> "error: Cannot parse struct or union!" >> >> Add support for parsing it. >> >> Signed-off-by: Aditya Srivastava >> --- >> scripts/kernel-doc | 12 ++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/kernel-doc b/scripts/kernel-doc >> index 8b5bc7bf4bb8..46e904dc3f87 100755 >> --- a/scripts/kernel-doc >> +++ b/scripts/kernel-doc >> @@ -1201,12 +1201,20 @@ sub dump_union($$) { >> sub dump_struct($$) { >> my $x = shift; >> my $file = shift; >> +my $decl_type; >> +my $members; >> >> if ($x =~ >> /(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/) >> { >> -my $decl_type = $1; >> +$decl_type = $1; >> $declaration_name = $2; >> -my $members = $3; >> +$members = $3; >> +} elsif ($x =~ >> /typedef\s+(struct|union)\s*\{(.*)\}(?:\s*(?:__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*\s*(\w*)\s*;/) >> { > > So this isn't your fault, but these regexes are really getting out of > hand. I would *really* like to see some effort made into making this > code more understandable / maintainable as we tweak this stuff. So: > > - Splitting out the common part, as suggested by Lukas, would be really >useful. That would also avoid the problem of only occurrence being >edited the next tine we add a new qualifier. > > - Splitting out other subsections of the regex and giving them symbolic >names would also help. > > - We really could use some comments before these branches saying what >they are doing; it is *not* obvious from the code. > > See what I'm getting at here? > Yep. Thanks for the feedback Lukas and Jonathan. I'll get back with a v2 for the patch. Thanks Aditya
Re: [RFC] scripts: kernel-doc: fix typedef support for struct parsing
Aditya Srivastava writes: > There are files in kernel, which use 'typedef struct' syntax for defining > struct. For eg, include/linux/zstd.h, drivers/scsi/megaraid/mega_common.h, > etc. > However, kernel-doc still does not support it, causing a parsing error. > > For eg, running scripts/kernel-doc -none on include/linux/zstd.h emits: > "error: Cannot parse struct or union!" > > Add support for parsing it. > > Signed-off-by: Aditya Srivastava > --- > scripts/kernel-doc | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/scripts/kernel-doc b/scripts/kernel-doc > index 8b5bc7bf4bb8..46e904dc3f87 100755 > --- a/scripts/kernel-doc > +++ b/scripts/kernel-doc > @@ -1201,12 +1201,20 @@ sub dump_union($$) { > sub dump_struct($$) { > my $x = shift; > my $file = shift; > +my $decl_type; > +my $members; > > if ($x =~ > /(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/) > { > - my $decl_type = $1; > + $decl_type = $1; > $declaration_name = $2; > - my $members = $3; > + $members = $3; > +} elsif ($x =~ > /typedef\s+(struct|union)\s*\{(.*)\}(?:\s*(?:__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*\s*(\w*)\s*;/) > { So this isn't your fault, but these regexes are really getting out of hand. I would *really* like to see some effort made into making this code more understandable / maintainable as we tweak this stuff. So: - Splitting out the common part, as suggested by Lukas, would be really useful. That would also avoid the problem of only occurrence being edited the next tine we add a new qualifier. - Splitting out other subsections of the regex and giving them symbolic names would also help. - We really could use some comments before these branches saying what they are doing; it is *not* obvious from the code. See what I'm getting at here? Thanks, jon
Re: [RFC] scripts: kernel-doc: fix typedef support for struct parsing
On Mon, Feb 22, 2021 at 5:03 PM Aditya Srivastava wrote: > > There are files in kernel, which use 'typedef struct' syntax for defining stylistic: s/in kernel/in the kernel tree/ s/defining struct/defining some struct/ > struct. For eg, include/linux/zstd.h, drivers/scsi/megaraid/mega_common.h, > etc. stylistic: s/For eg/E.g./ or s/For eg/For example/ semantically: It makes much more sense to name how many occurrences of this pattern exist in the kernel tree within how many files in order to get a good impression on the use of that pattern within the kernel tree. > However, kernel-doc still does not support it, causing a parsing error. > > For eg, running scripts/kernel-doc -none on include/linux/zstd.h emits: > "error: Cannot parse struct or union!" > semantically: Drop the example and turn the two sentences above into one. > Add support for parsing it. > > Signed-off-by: Aditya Srivastava > --- > scripts/kernel-doc | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/scripts/kernel-doc b/scripts/kernel-doc > index 8b5bc7bf4bb8..46e904dc3f87 100755 > --- a/scripts/kernel-doc > +++ b/scripts/kernel-doc > @@ -1201,12 +1201,20 @@ sub dump_union($$) { > sub dump_struct($$) { > my $x = shift; > my $file = shift; > +my $decl_type; > +my $members; > > if ($x =~ > /(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/) > { > - my $decl_type = $1; > + $decl_type = $1; > $declaration_name = $2; > - my $members = $3; > + $members = $3; > +} elsif ($x =~ > /typedef\s+(struct|union)\s*\{(.*)\}(?:\s*(?:__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*\s*(\w*)\s*;/) > { > + $decl_type = $1; > + $declaration_name = $3; > + $members = $2; > +} > Could you put the common expression parts into meaningful variables to avoid repeating yourself in the two pattern matching expressions? Other than that, it looks good to me. Lukas > +if ($members) { > if ($identifier ne $declaration_name) { > print STDERR "${file}:$.: warning: expecting prototype for > $decl_type $identifier. Prototype was for $decl_type $declaration_name > instead\n"; > return; > -- > 2.17.1 >
[RFC] scripts: kernel-doc: fix typedef support for struct parsing
There are files in kernel, which use 'typedef struct' syntax for defining struct. For eg, include/linux/zstd.h, drivers/scsi/megaraid/mega_common.h, etc. However, kernel-doc still does not support it, causing a parsing error. For eg, running scripts/kernel-doc -none on include/linux/zstd.h emits: "error: Cannot parse struct or union!" Add support for parsing it. Signed-off-by: Aditya Srivastava --- scripts/kernel-doc | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/kernel-doc b/scripts/kernel-doc index 8b5bc7bf4bb8..46e904dc3f87 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -1201,12 +1201,20 @@ sub dump_union($$) { sub dump_struct($$) { my $x = shift; my $file = shift; +my $decl_type; +my $members; if ($x =~ /(struct|union)\s+(\w+)\s*\{(.*)\}(\s*(__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*/) { - my $decl_type = $1; + $decl_type = $1; $declaration_name = $2; - my $members = $3; + $members = $3; +} elsif ($x =~ /typedef\s+(struct|union)\s*\{(.*)\}(?:\s*(?:__packed|__aligned|cacheline_aligned_in_smp|cacheline_aligned|__attribute__\s*\(\([a-z0-9,_\s\(\)]*\)\)))*\s*(\w*)\s*;/) { + $decl_type = $1; + $declaration_name = $3; + $members = $2; +} +if ($members) { if ($identifier ne $declaration_name) { print STDERR "${file}:$.: warning: expecting prototype for $decl_type $identifier. Prototype was for $decl_type $declaration_name instead\n"; return; -- 2.17.1