Re: [PATCH v2] parse: handle __cleanup__ attribute

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 08, 2023 at 12:49:34PM +0300, Dan Carpenter wrote:
> The kernel has recently started using the __cleanup__ attribute.  Save
> a pointer to cleanup function.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> v2:  The first version of this patch had a bug handling a list of
>      declarations.  I had to add a .cleanup = NULL at the start of
>      the loops iterations in declaration_list() and
>      external_declaration().

OK. See some notes here under, but first at all please forgive my very long delay.
 
> diff --git a/parse.c b/parse.c
> index 3d6fef7cb011..e5b5e6acc062 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -537,6 +542,7 @@ static struct init_keyword {
>  	/* Attributes */
>  	D("packed",		&packed_op),
>  	D("aligned",		&aligned_op),
> +	D("__cleanup__",	&cleanup_op),

This should simply be D("cleanup" (to accept both the plain form and the __X__ form).

> @@ -1964,6 +1984,7 @@ struct token *typename(struct token *token, struct symbol **p, int *forced)
>  	token = declarator(token, &ctx);
>  	apply_modifiers(token->pos, &ctx);
>  	sym->ctype = ctx.ctype;
> +	sym->cleanup = ctx.cleanup;

I don't think this should be needed because the cleanup attribute should be
'attached' to individual symbols, not their types (but I have no idea what GCC do).

> @@ -2924,6 +2945,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
>  
>  	decl->ctype = ctx.ctype;
>  	decl->ctype.modifiers |= mod;
> +	decl->cleanup = ctx.cleanup;

Similarly, the attribute should only be applied to automatic variables,
so this should not be needed/should be detected as an error.

> diff --git a/symbol.h b/symbol.h
> index 5270fcd73a10..88130c15d4bd 100644
> --- a/symbol.h
> +++ b/symbol.h
> @@ -204,6 +205,7 @@ struct symbol {
>  			struct statement *inline_stmt;
>  			struct symbol_list *inline_symbol_list;
>  			struct expression *initializer;
> +			struct expression *cleanup;

This annoys me a little bit as it adds one more member to struct symbol
which is already the biggest memory user.
But currently this is fine as other currently ignored attributes are also
concerned. A solution is needed for all of them but there is no urgency.

It would be nice to have some testcases though.

Anyway, I can take this patch as-is modulo the few changes here above or
you can send a v3 if you prefer. But it may take a few days before I'm able
to push it to k.org.

Best regards,
-- Luc





[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux