diff mbox series

[2/6] inline: comment about creating node of node on variadics

Message ID 20220626130748.74163-3-lucvoo@kernel.org (mailing list archive)
State Mainlined, archived
Headers show
Series cleanup related to inlining of variadic functions | expand

Commit Message

Luc Van Oostenryck June 26, 2022, 1:07 p.m. UTC
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

When inlining a variadic function (unsupported in general by
sparse but OK when the arguments are unused and occurs as such
in the kernel), the extra arguments are added in the declaration
list as SYM_NODE.

But these arguments can already be SYM_NODEs. Sparse doesn't
support everywhere such nested nodes (they must be merged) but
in this case it's fine as the node will be merged when evaluated.

Add a comment telling the situation is fine.
Also, move the code to where the variadic arguments are handled
since the fixed one will be anyway directly overwritten.

Note: Sparse doesn't really support inlining of variadic functions
      but is fine when the arguments are not used (and such cases
      occur in the kernel).

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 inline.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Ramsay Jones June 26, 2022, 2:42 p.m. UTC | #1
On 26/06/2022 14:07, Luc Van Oostenryck wrote:
> From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> 
> When inlining a variadic function (unsupported in general by
> sparse but OK when the arguments are unused and occurs as such
> in the kernel), the extra arguments are added in the declaration
> list as SYM_NODE.
> 
> But these arguments can already be SYM_NODEs. Sparse doesn't
> support everywhere such nested nodes (they must be merged) but
> in this case it's fine as the node will be merged when evaluated.
> 
> Add a comment telling the situation is fine.
> Also, move the code to where the variadic arguments are handled
> since the fixed one will be anyway directly overwritten.
> 
> Note: Sparse doesn't really support inlining of variadic functions
>       but is fine when the arguments are not used (and such cases
>       occur in the kernel).

This note prompted a feeling of deja-vu :) It is simply repeating
(in slightly different words) the content of the first paragraph.

ATB,
Ramsay Jones

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  inline.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/inline.c b/inline.c
> index 0097e4bf620a..d031c9b19128 100644
> --- a/inline.c
> +++ b/inline.c
> @@ -542,11 +542,15 @@ int inline_function(struct expression *expr, struct symbol *sym)
>  	FOR_EACH_PTR(arg_list, arg) {
>  		struct symbol *a = alloc_symbol(arg->pos, SYM_NODE);
>  
> -		a->ctype.base_type = arg->ctype;
>  		if (name) {
>  			*a = *name;
>  			set_replace(name, a);
>  			add_symbol(&fn_symbol_list, a);
> +		} else {
> +			// This may create a node of a node but it will
> +			// be resolved later when the corresponding
> +			// STMT_DECLARATION will be evaluated.
> +			a->ctype.base_type = arg->ctype;
>  		}
>  		a->initializer = arg;
>  		add_symbol(&arg_decl, a);
Luc Van Oostenryck June 26, 2022, 4:07 p.m. UTC | #2
On Sun, Jun 26, 2022 at 03:42:30PM +0100, Ramsay Jones wrote:
> 
> 
> On 26/06/2022 14:07, Luc Van Oostenryck wrote:
> > From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > 
> > When inlining a variadic function (unsupported in general by
> > sparse but OK when the arguments are unused and occurs as such
> > in the kernel), the extra arguments are added in the declaration
> > list as SYM_NODE.
> > 
> > But these arguments can already be SYM_NODEs. Sparse doesn't
> > support everywhere such nested nodes (they must be merged) but
> > in this case it's fine as the node will be merged when evaluated.
> > 
> > Add a comment telling the situation is fine.
> > Also, move the code to where the variadic arguments are handled
> > since the fixed one will be anyway directly overwritten.
> > 
> > Note: Sparse doesn't really support inlining of variadic functions
> >       but is fine when the arguments are not used (and such cases
> >       occur in the kernel).
> 
> This note prompted a feeling of deja-vu :) It is simply repeating
> (in slightly different words) the content of the first paragraph.

Hehe, indeed. I'm really bad at rereading myself.

Thanks for noticing.
-- Luc
diff mbox series

Patch

diff --git a/inline.c b/inline.c
index 0097e4bf620a..d031c9b19128 100644
--- a/inline.c
+++ b/inline.c
@@ -542,11 +542,15 @@  int inline_function(struct expression *expr, struct symbol *sym)
 	FOR_EACH_PTR(arg_list, arg) {
 		struct symbol *a = alloc_symbol(arg->pos, SYM_NODE);
 
-		a->ctype.base_type = arg->ctype;
 		if (name) {
 			*a = *name;
 			set_replace(name, a);
 			add_symbol(&fn_symbol_list, a);
+		} else {
+			// This may create a node of a node but it will
+			// be resolved later when the corresponding
+			// STMT_DECLARATION will be evaluated.
+			a->ctype.base_type = arg->ctype;
 		}
 		a->initializer = arg;
 		add_symbol(&arg_decl, a);