From: Dan Carpenter <dan.carpenter@linaro.org>
To: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc: linux-sparse@vger.kernel.org
Subject: Re: [PATCH v2] parse: handle __cleanup__ attribute
Date: Tue, 12 Dec 2023 12:39:40 +0300 [thread overview]
Message-ID: <44e22df1-734e-49c5-b20b-4f4cdbce24a6@suswa.mountain> (raw)
In-Reply-To: <i4s2gnr6rlq3yhmfiy7lkbsahnaioilksvmx3eocdjfh2434zo@zhxhwwgqpxt2>
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@linaro.org>
> > ---
> > 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
next prev parent reply other threads:[~2023-12-12 9:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-08 9:49 [PATCH v2] parse: handle __cleanup__ attribute Dan Carpenter
2023-12-11 13:16 ` Luc Van Oostenryck
2023-12-12 9:39 ` Dan Carpenter [this message]
2023-12-13 10:14 ` Dan Carpenter
2023-12-14 13:05 ` Luc Van Oostenryck
2023-12-14 13:20 ` Dan Carpenter
2023-12-18 13:51 ` Luc Van Oostenryck
2024-01-17 20:20 ` Dmitry Torokhov
2024-02-29 14:03 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=44e22df1-734e-49c5-b20b-4f4cdbce24a6@suswa.mountain \
--to=dan.carpenter@linaro.org \
--cc=linux-sparse@vger.kernel.org \
--cc=luc.vanoostenryck@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).