All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Elfring <Markus.Elfring@web.de>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org
Subject: Re: coccinelle: adjustments for array.cocci?
Date: Sun, 17 Nov 2019 08:56:10 +0100	[thread overview]
Message-ID: <57b5d1c9-72c1-6fff-a242-90f5f24f0972@web.de> (raw)
In-Reply-To: <fc56b970-4ca1-7734-c4bb-f57cae7a273f@web.de>

>> @@
>> expression dst, src, n, E;
>> @@
>>  memcpy(dst, src, sizeof(
>> +                        *(
>>                            E
>> -                           [...]
>> +                         )
>>                          ) * n
>>        )
>
> That's longer and looks more complicated to me

I point another possibility out to express a change specification
by the means of the semantic patch language.
How would you think about such SmPL code if the indentation
will be reduced?


> 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 show other development preferences occasionally.


> I can see that indenting the sizeof parameter and parentheses could
> improve readability, though.

Thanks that you can follow such coding style aspects.


>> @@
>> 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).

Would you become keen to find the reasons out for unexpected data processing
results (also by the software combination “Coccinelle 1.0.8-00004-g842075f7”)
at this place?

But this transformation rule can probably be omitted if the usage
of SmPL disjunctions will be increased in a subsequent rule, can't it?


>> @@
>> 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.

Would you like to use the SmPL code “*( \( src_ptr \| src_arr \) )” instead?


>> @@
>> 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;

These functions provide another programming interface.


> why would we want to jam transformations for them into the same rule
> like this?

Possible nicer run time characteristics by the Coccinelle software.


> The only overlap seems to be n.

These case distinctions can share also the metavariable “T” for the
desired source code deletion.


> Handling memmove/MOVE_ARRAY and memcpy/COPY_ARRAY together would make
> more sense, as they take the same kinds of parameters.

Would you like to adjust the SmPL code in such a design direction?


> I didn't know that disjunctions can be specified inline using \(, \|
> and \), though.  Rules can be much more compact that way.

I hope that more corresponding software improvements can be achieved.


> Mixing languages like that can also be quite confusing.

I agree to this development concern.


>> 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.

But how does the software situation look like if the original source code
would contain coding style issues?

It seems to be possible to specify SmPL code in a way so that even questionable
code layout would be preserved by an automatic transformation.

Regards,
Markus

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

Thread overview: 50+ 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 11:11                   ` [Cocci] " Markus Elfring
2019-11-15 14:20                   ` Markus Elfring
2019-11-15 14:20                     ` [Cocci] " Markus Elfring
2019-11-15 18:50                   ` Markus Elfring
2019-11-15 18:50                     ` [Cocci] " Markus Elfring
2019-11-16  1:00                     ` Julia Lawall
2019-11-16  1:00                       ` Julia Lawall
2019-11-16  6:57                       ` Markus Elfring
2019-11-16  6:57                         ` Markus Elfring
2019-11-16  8:29                       ` Markus Elfring
2019-11-16  8:29                         ` Markus Elfring
2019-11-16 17:57                   ` Julia Lawall
2019-11-16 17:57                     ` Julia Lawall
2019-11-16 18:29                     ` Markus Elfring
2019-11-16 18:29                       ` Markus Elfring
2019-11-15 20:37   ` coccinelle: " Markus Elfring
2019-11-16 21:13     ` René Scharfe
2019-11-17  7:56       ` Markus Elfring [this message]
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
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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57b5d1c9-72c1-6fff-a242-90f5f24f0972@web.de \
    --to=markus.elfring@web.de \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /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.