The same script kernel-doc is used by the U-Boot project.
kernel-doc fails to parse function definitions like the one below
efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl,
void (EFIAPI *notify_function) (
struct efi_event *event,
void *context),
void *notify_context, efi_guid_t *group,
struct efi_event **event)
{
due to the "EFIAPI" attribute preceding the function name.
Signed-off-by: Heinrich Schuchardt <[email protected]>
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 31a34ced55a3..597e3223b791 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1381,7 +1381,7 @@ sub create_parameterlist($$$$) {
} elsif ($arg =~ m/\(.+\)\s*\(/) {
# pointer-to-function
$arg =~ tr/#/,/;
- $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
+ $arg =~ m/[^\(]+\([\w\s]*\*?\s*([\w\.]*)\s*\)/;
$param = $1;
$type = $arg;
$type =~ s/([^\(]+\(\*?)\s*$param/$1/;
--
2.18.0
On Mon, 3 Sep 2018 20:41:53 +0200
Heinrich Schuchardt <[email protected]> wrote:
> The same script kernel-doc is used by the U-Boot project.
Interesting, I didn't know that. Please pass on my condolences :)
>
> kernel-doc fails to parse function definitions like the one below
>
> efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl,
> void (EFIAPI *notify_function) (
> struct efi_event *event,
> void *context),
> void *notify_context, efi_guid_t *group,
> struct efi_event **event)
> {
>
> due to the "EFIAPI" attribute preceding the function name.
This is a good description of the problem; a proper changelog should
really say what the patch *does* about the problem too. Especially
since...
> Signed-off-by: Heinrich Schuchardt <[email protected]>
> ---
> scripts/kernel-doc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 31a34ced55a3..597e3223b791 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1381,7 +1381,7 @@ sub create_parameterlist($$$$) {
> } elsif ($arg =~ m/\(.+\)\s*\(/) {
> # pointer-to-function
> $arg =~ tr/#/,/;
> - $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
> + $arg =~ m/[^\(]+\([\w\s]*\*?\s*([\w\.]*)\s*\)/;
The meaning of this change doesn't just jump off the screen, even for
folks who are accustomed to reading regexes. It would be nice to explain
what is actually being changed and what the expected new behavior is. It
appears to be consuming any normal text/white space prior to the optional
"*". Are you sure it doesn't overshoot and consume too much if the *
isn't there?
Thanks,
jon
On 09/03/2018 09:00 PM, Jonathan Corbet wrote:
> On Mon, 3 Sep 2018 20:41:53 +0200
> Heinrich Schuchardt <[email protected]> wrote:
>
>> The same script kernel-doc is used by the U-Boot project.
>
> Interesting, I didn't know that. Please pass on my condolences :)
>>
>> kernel-doc fails to parse function definitions like the one below
>>
>> efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl,
>> void (EFIAPI *notify_function) (
>> struct efi_event *event,
>> void *context),
>> void *notify_context, efi_guid_t *group,
>> struct efi_event **event)
>> {
>>
>> due to the "EFIAPI" attribute preceding the function name.
>
> This is a good description of the problem; a proper changelog should
> really say what the patch *does* about the problem too. Especially
> since...
>
Thanks for reviewing.
>> Signed-off-by: Heinrich Schuchardt <[email protected]>
>> ---
>> scripts/kernel-doc | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
>> index 31a34ced55a3..597e3223b791 100755
>> --- a/scripts/kernel-doc
>> +++ b/scripts/kernel-doc
>> @@ -1381,7 +1381,7 @@ sub create_parameterlist($$$$) {
>> } elsif ($arg =~ m/\(.+\)\s*\(/) {
This matches anything that is not even remotely a function pointer, e.g.
"( )(".
>> # pointer-to-function
>> $arg =~ tr/#/,/;
>> - $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
^
Here we allow for 0..1 asterixes.
If there is no asterix it is not a function pointer. Why should we care
for this case?
We do not allow for a space preceding the asterix, though it is
perfectly legal (though it is not the preferred formatting).
Probably we should start with collecting the different test cases that
the regex have to encompass. I already see the following possible
parameters:
int (*f)(int a)
int (* f)(int a)
int ( *f)(int a)
int ( * f)(int a)
int (FOO *f)(int a)
int (__attrib__((ms_abi)) *f)(int a)
int (*f(int b))(int a)
Which other cases do you see?
Best regards
Heinrich
>> + $arg =~ m/[^\(]+\([\w\s]*\*?\s*([\w\.]*)\s*\)/;
>
> The meaning of this change doesn't just jump off the screen, even for
> folks who are accustomed to reading regexes. It would be nice to explain
> what is actually being changed and what the expected new behavior is. It
> appears to be consuming any normal text/white space prior to the optional
> "*". Are you sure it doesn't overshoot and consume too much if the *
> isn't there?
On Mon, 3 Sep 2018 22:29:00 +0200
Heinrich Schuchardt <[email protected]> wrote:
> >> # pointer-to-function
> >> $arg =~ tr/#/,/;
> >> - $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
>
> m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
> ^
> Here we allow for 0..1 asterixes.
>
> If there is no asterix it is not a function pointer. Why should we care
> for this case?
GCC seems to allow that asterisk (asterix is an indomitable Gaul :) to be
missing; not sure if that's officially allowed by the language or not. I
also don't know if any code in the kernel elides it, but *somebody* at
some point made it optional, presumably with some reason. It would be
instructive to take out that "?" and see what changes happen in a docs
build; I'll try to find a moment to do that.
jon
On Mon, 2018-09-03 at 15:38 -0600, Jonathan Corbet wrote:
> On Mon, 3 Sep 2018 22:29:00 +0200
> Heinrich Schuchardt <[email protected]> wrote:
>
> > > > # pointer-to-function
> > > > $arg =~ tr/#/,/;
> > > > - $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
> >
> > m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
> > ^
> > Here we allow for 0..1 asterixes.
> >
> > If there is no asterix it is not a function pointer. Why should we care
> > for this case?
>
> GCC seems to allow that asterisk (asterix is an indomitable Gaul :) to be
> missing; not sure if that's officially allowed by the language or not. I
> also don't know if any code in the kernel elides it,
Many typedefs for function pointers do not use the *
Dunno if there are many others, I didn't look hard.
$ git grep -P '\w+\s*\*?\s*\(\s*\w+\w*\)\s*\(\w+' | \
grep -w typedef
On 04/09/18 00:28, Joe Perches wrote:
> On Mon, 2018-09-03 at 15:38 -0600, Jonathan Corbet wrote:
>> On Mon, 3 Sep 2018 22:29:00 +0200
>> Heinrich Schuchardt <[email protected]> wrote:
>>
>>>>> # pointer-to-function
>>>>> $arg =~ tr/#/,/;
>>>>> - $arg =~ m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
>>>
>>> m/[^\(]+\(\*?\s*([\w\.]*)\s*\)/;
>>> ^
>>> Here we allow for 0..1 asterixes.
What's with pointers to function pointers and/or function pointer
arrays? ;-)
>>> If there is no asterix it is not a function pointer. Why should we care
>>> for this case?
>>
>> GCC seems to allow that asterisk (asterix is an indomitable Gaul :) to be
*g*
>> missing; not sure if that's officially allowed by the language or not. I
>> also don't know if any code in the kernel elides it,
>
> Many typedefs for function pointers do not use the *
> Dunno if there are many others, I didn't look hard.
---- snip ----
typedef void (*foo)(int);
---- snip ----
is a function pointer type (and nothing else;-) and can be used to
declare and/or define a pointer to a function as in
---- snip ----
foo ptr_to_func_with_int_param_and_no_result;
---- snip ----
---- snip ----
typedef void bar(int);
---- snip ----
is a function type and can be used for function prototypes as in
---- snip ----
bar func_with_int_param_and_no_result;
---- snip ----
and/or function pointers as in
---- snip ----
bar *also_ptr_to_func_with_int_param_and_no_result;
---- snip ----
Both of work (in functions) as
---- snip ----
ptr_to_func_with_int_param_and_no_result =
&func_with_int_param_and_no_result;
ptr_to_func_with_int_param_and_no_result =
func_with_int_param_and_no_result;
also_ptr_to_func_with_int_param_and_no_result =
&func_with_int_param_and_no_result;
also_ptr_to_func_with_int_param_and_no_result =
func_with_int_param_and_no_result;
---- snip ----
Note the missing "()" in both cases (otherwise it would be a function
call - possibly returning the function pointers).
I don't know if there is a preferred style in the kernel though.
MfG,
Bernd
--
Bernd Petrovitsch Email : [email protected]
LUGA : http://www.luga.at