From: "René Scharfe" <l.s.r@web.de>
To: Markus Elfring <Markus.Elfring@web.de>
Cc: git@vger.kernel.org
Subject: Re: coccinelle: adjustments for array.cocci?
Date: Sat, 16 Nov 2019 22:13:45 +0100 [thread overview]
Message-ID: <fc56b970-4ca1-7734-c4bb-f57cae7a273f@web.de> (raw)
In-Reply-To: <75b9417b-14a7-c9c6-25eb-f6e05f340376@web.de>
Am 15.11.19 um 21:37 schrieb Markus Elfring:
>> This eliminates duplication in the semantic patch, which is good.
>
> Thanks that you think in such a direction.
>
>
>> It messes up the indentation of n in some of the cases in 921d49be86 ("use
>> COPY_ARRAY for copying arrays", 2019-06-15), though. Hmm, but that can
>> be cured by duplicating the comma:
>
> I have picked up further improvement possibilities for this SmPL script.
> Would you like to integrate any of these changes?
Not sure, could you please elaborate on the benefits of each proposed
change?
> @@
> expression dst, src, n, E;
> @@
> memcpy(dst, src, sizeof(
> + *(
> E
> - [...]
> + )
> ) * n
> )
That's longer and looks more complicated to me than what we currently have:
@@
expression dst, src, n, E;
@@
memcpy(dst, src, n * sizeof(
- E[...]
+ *(E)
))
Avoiding to duplicate E doesn't seem to be worth it. I can see that
indenting the sizeof parameter and parentheses could improve readability,
though.
> @@
> type T;
> T *ptr;
> T[] arr;
> expression E, n;
> @@
> memcpy(
> ( ptr, E, sizeof(
> - *(ptr)
> + T
> ) * n
> | arr, E, sizeof(
> - *(arr)
> + T
> ) * n
> | E, ptr, sizeof(
> - *(ptr)
> + T
> ) * n
> | E, arr, sizeof(
> - *(arr)
> + T
> ) * n
> )
> )
This still fails to regenerate two of the changes from 921d49be86 (use
COPY_ARRAY for copying arrays, 2019-06-15), at least with for me (and
Coccinelle 1.0.4).
> @@
> type T;
> T* dst_ptr, src_ptr;
> T[] dst_arr, src_arr;
> expression n, x;
> @@
> -memcpy
> +COPY_ARRAY
> (
> ( dst_ptr
> | dst_arr
> )
> ,
> ( src_ptr
> | src_arr
> )
> - , (n) * \( sizeof(T) \| sizeof(*(x)) \)
> + , n
> )
That x could be anything -- it's not tied to the element size of source
or destination. Such a transformation might change the meaning of the
code, as COPY_ARRAY will use the element size of the destination behind
the scenes. So that doesn't look safe to me.
> @@
> type T;
> T* dst, src, ptr;
> expression n;
> @@
> (
> -memmove
> +MOVE_ARRAY
> (dst, src
> - , (n) * \( sizeof(* \( dst \| src \) ) \| sizeof(T) \)
> + , n
> )
> |
> -ptr = xmalloc((n) * \( sizeof(*ptr) \| sizeof(T) \))
> +ALLOC_ARRAY(ptr, n)
> );
memmove/MOVE_ARRAY and xmalloc/ALLOC_ARRAY are quite different; why
would we want to jam transformations for them into the same rule like
this? The only overlap seems to be n. Handling memmove/MOVE_ARRAY and
memcpy/COPY_ARRAY together would make more sense, as they take the same
kinds of parameters.
I didn't know that disjunctions can be specified inline using \(, \|
and \), though. Rules can be much more compact that way. Mixing
languages like that can also be quite confusing. Syntax highlighting
could help; https://github.com/ahf/cocci-syntax at least doesn't
show those any different from regular code, though.
> Now I observe that the placement of space characters can be a coding style
> concern at four places for adjusted lines by the generated patch.
> Would you like to clarify remaining issues for pretty-printing
> in such use cases?
Ideally, generated code should adhere to Documentation/CodingGuidelines,
so that it can be accepted without requiring hand-editing.
René
next prev parent reply other threads:[~2019-11-16 21:13 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-12 15:08 coccinelle: adjustments for array.cocci? Markus Elfring
2019-11-12 18:37 ` René Scharfe
2019-11-13 2:11 ` Junio C Hamano
2019-11-13 8:49 ` Markus Elfring
2019-11-14 2:03 ` Junio C Hamano
2019-11-14 13:15 ` Markus Elfring
2019-11-14 16:41 ` René Scharfe
2019-11-14 17:14 ` Markus Elfring
2019-11-14 17:46 ` René Scharfe
2019-11-15 11:11 ` git-coccinelle: " Markus Elfring
2019-11-15 14:20 ` Markus Elfring
2019-11-15 18:50 ` Markus Elfring
2019-11-16 1:00 ` [Cocci] " Julia Lawall
2019-11-16 6:57 ` Markus Elfring
2019-11-16 8:29 ` Markus Elfring
2019-11-16 17:57 ` Julia Lawall
2019-11-16 18:29 ` Markus Elfring
2019-11-15 20:37 ` coccinelle: " Markus Elfring
2019-11-16 21:13 ` René Scharfe [this message]
2019-11-17 7:56 ` Markus Elfring
2019-11-17 13:40 ` René Scharfe
2019-11-17 18:19 ` Markus Elfring
2019-11-19 19:14 ` René Scharfe
2019-11-19 20:21 ` Markus Elfring
2019-11-21 19:01 ` René Scharfe
2019-11-16 16:33 ` Markus Elfring
2019-11-16 21:38 ` René Scharfe
2019-11-17 8:19 ` Markus Elfring
2019-11-17 13:40 ` René Scharfe
2019-11-17 18:36 ` Markus Elfring
2019-11-19 19:15 ` René Scharfe
2019-11-18 16:10 ` [PATCH] coccinelle: improve array.cocci Markus Elfring
2019-11-19 19:15 ` René Scharfe
2019-11-20 9:01 ` Markus Elfring
2019-11-21 19:02 ` René Scharfe
2019-11-21 19:44 ` Markus Elfring
2019-11-22 15:29 ` SZEDER Gábor
2019-11-22 16:17 ` Markus Elfring
2019-11-22 5:54 ` [PATCH] " Junio C Hamano
2019-11-22 7:34 ` Markus Elfring
2020-01-25 8:23 ` Markus Elfring
-- strict thread matches above, loose matches on Subject: below --
2019-11-12 15:08 coccinelle: adjustments for array.cocci? Markus Elfring
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc56b970-4ca1-7734-c4bb-f57cae7a273f@web.de \
--to=l.s.r@web.de \
--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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).