* [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers
@ 2018-09-03 18:41 Heinrich Schuchardt
2018-09-03 19:00 ` Jonathan Corbet
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2018-09-03 18:41 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-doc, linux-kernel, Heinrich Schuchardt
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 <xypron.glpk@gmx.de>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers
2018-09-03 18:41 [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers Heinrich Schuchardt
@ 2018-09-03 19:00 ` Jonathan Corbet
2018-09-03 20:29 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Corbet @ 2018-09-03 19:00 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: linux-doc, linux-kernel
On Mon, 3 Sep 2018 20:41:53 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> 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 <xypron.glpk@gmx.de>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers
2018-09-03 19:00 ` Jonathan Corbet
@ 2018-09-03 20:29 ` Heinrich Schuchardt
2018-09-03 21:38 ` Jonathan Corbet
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2018-09-03 20:29 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-doc, linux-kernel
On 09/03/2018 09:00 PM, Jonathan Corbet wrote:
> On Mon, 3 Sep 2018 20:41:53 +0200
> Heinrich Schuchardt <xypron.glpk@gmx.de> 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 <xypron.glpk@gmx.de>
>> ---
>> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers
2018-09-03 20:29 ` Heinrich Schuchardt
@ 2018-09-03 21:38 ` Jonathan Corbet
2018-09-03 22:28 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Corbet @ 2018-09-03 21:38 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: linux-doc, linux-kernel
On Mon, 3 Sep 2018 22:29:00 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers
2018-09-03 21:38 ` Jonathan Corbet
@ 2018-09-03 22:28 ` Joe Perches
2018-09-03 22:54 ` Bernd Petrovitsch
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2018-09-03 22:28 UTC (permalink / raw)
To: Jonathan Corbet, Heinrich Schuchardt; +Cc: linux-doc, linux-kernel
On Mon, 2018-09-03 at 15:38 -0600, Jonathan Corbet wrote:
> On Mon, 3 Sep 2018 22:29:00 +0200
> Heinrich Schuchardt <xypron.glpk@gmx.de> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers
2018-09-03 22:28 ` Joe Perches
@ 2018-09-03 22:54 ` Bernd Petrovitsch
0 siblings, 0 replies; 6+ messages in thread
From: Bernd Petrovitsch @ 2018-09-03 22:54 UTC (permalink / raw)
To: Joe Perches, Jonathan Corbet, Heinrich Schuchardt; +Cc: linux-doc, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2290 bytes --]
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 <xypron.glpk@gmx.de> 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 : bernd@petrovitsch.priv.at
LUGA : http://www.luga.at
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1809 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-03 22:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-03 18:41 [PATCH 1/1] docs: kernel-doc: fix parsing of function pointers Heinrich Schuchardt
2018-09-03 19:00 ` Jonathan Corbet
2018-09-03 20:29 ` Heinrich Schuchardt
2018-09-03 21:38 ` Jonathan Corbet
2018-09-03 22:28 ` Joe Perches
2018-09-03 22:54 ` Bernd Petrovitsch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.