All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Markus Elfring <Markus.Elfring@web.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] coccinelle: merge two rules from flex_alloc.cocci
Date: Tue, 12 Nov 2019 09:59:26 -0800	[thread overview]
Message-ID: <20191112175926.GA41101@generichostname> (raw)
In-Reply-To: <f867512c-e5b2-6bca-2a37-2976f4c182bd@web.de>

Hi Markus,

Thanks for the contribution.

I see that you've sent many Coccinelle patches to the mailing list. It
might be better to send them all together as a single threaded patchset
so that reviewers will have an easier time finding all of them.

On Tue, Nov 12, 2019 at 04:34:34PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 12 Nov 2019 16:30:14 +0100
> 
> This script contained two transformation rules for the semantic patch language
> which used duplicate code.
> Thus combine these rules by using a SmPL disjunction for the replacement
> of two identifiers.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  contrib/coccinelle/flex_alloc.cocci | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/contrib/coccinelle/flex_alloc.cocci b/contrib/coccinelle/flex_alloc.cocci
> index e9f7f6d861..1b4fa8f801 100644
> --- a/contrib/coccinelle/flex_alloc.cocci
> +++ b/contrib/coccinelle/flex_alloc.cocci
> @@ -1,13 +1,14 @@
> -@@
> +@adjustment@

None of our other cocci scripts have rulenames so I would drop the
rulename here. It also doesn't really help since its name is so generic.

I would also echo this for the other patches you've sent.

>  expression str;
> -identifier x, flexname;
> -@@
> -- FLEX_ALLOC_MEM(x, flexname, str, strlen(str));
> -+ FLEX_ALLOC_STR(x, flexname, str);
> -
> -@@
> -expression str;
> -identifier x, ptrname;
> -@@
> -- FLEXPTR_ALLOC_MEM(x, ptrname, str, strlen(str));
> -+ FLEXPTR_ALLOC_STR(x, ptrname, str);
> +identifier x, name;
> +@@
> +(
> +-FLEX_ALLOC_MEM
> ++FLEX_ALLOC_STR
> +|
> +-FLEXPTR_ALLOC_MEM
> ++FLEXPTR_ALLOC_STR
> +)
> +               (x, name, str
> +-                           , strlen(str)
> +               );

Small nitpick but to be inline with how the rest of our cocci scripts
are written, I'd write this as

	  (x, name, str
	- , strlen(str)
	  );

Thanks,

Denton

> --
> 2.24.0
> 

  reply	other threads:[~2019-11-12 17:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-12 15:34 [PATCH] coccinelle: merge two rules from flex_alloc.cocci Markus Elfring
2019-11-12 17:59 ` Denton Liu [this message]
2019-11-13 18:27   ` Martin Ågren
2019-11-13 21:10     ` Markus Elfring
2019-11-14  6:37       ` Martin Ågren
2019-11-14  8:15         ` Markus Elfring
2019-11-14 16:35           ` SZEDER Gábor
2019-11-14 17:30             ` Markus Elfring
2019-11-15  5:13             ` Junio C Hamano

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=20191112175926.GA41101@generichostname \
    --to=liu.denton@gmail.com \
    --cc=Markus.Elfring@web.de \
    --cc=git@vger.kernel.org \
    /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 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.