Am 10.03.2017 um 18:57 schrieb Jeff King:
On Fri, Mar 10, 2017 at 05:20:13PM +0100, René Scharfe wrote:
I think this misses the other two cases: (*dst, src) and (*dst, *src).
... and that's why I left them out. You can't get dst vs. *dst wrong with
structs (at least not without the compiler complaining); only safe
transformations are included in this round.
I don't think the transformation could be wrong without the original
code being wrong.
Avoiding to introduce bugs with automatic transformations is essential,
indeed -- if we're not careful here we'd be better off keeping the
original code.
I'm also not sure why mine would be unsafe and yours would be safe. It
seems like the other way around, because mine will do:
*dst = ...
which would fail unless "dst" is a pointer. Whereas in yours, we end up
with:
dst = ...
and somebody mistaking pointer assignment for a struct copy would not
notice.
If dst is a struct then having *dst on the left-hand side results in a
compiler error -- you can't get that one wrong. If it's a pointer then
both dst and *dst can be valid (pointer assignment vs. content copy), so
there is the possibility of making a mistake without the compiler noticing.
But either way, I don't think we can have a rule like "you can use
struct assignment only if you don't have a pointer for the destination".
That's too arcane to expect developers and reviewers to follow. We
should either allow struct assignment or not.
With an automatic transformation in place it's more like "you can't use
memcpy() in this case any more as it gets turned into an assignment with
the next cocci patch". I think we shouldn't be that restrictive for
pointers as destinations (yet?).
René