On Mon, Dec 11, 2023 at 02:16:49PM +0100, Luc Van Oostenryck wrote:
> 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.
>
I would demand my money back, but I checked with my accountant and it
turns out I'm not paying you anything. :P
> > 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.
>
Yeah. There are a couple other "cleanup" lines later in the function
that should be deleted as well, I see.
Let me test this out resend in a few days.
regards,
dan carpenter
