Thread (2 messages) flat view 2 messages, 2 authors, 2017-01-31

Re: [PATCH 1/5] add SWAP macro

From: Junio C Hamano <hidden>
Date: 2017-01-31 22:30:03

Jeff King [off-list ref] writes:
... I wonder if it would be more natural for it to take
pointers-to-objects, making it look more like a real function (i.e.,
SWAP(&a, &b) instead of SWAP(a, b)". And then these funny corner cases
become quite obvious in the caller, because the caller is the one who
has to type "&".
Hmmmm.  

While this looks very attractive in theory by forcing 'a' and 'b' to
be lvalues, it probably invites mistakes go unnoticed during the
review when the code wants to swap two pointer variables.  

For example,

apply.c:            SWAP(p->new_name, p->old_name);

is now a bug and will swap only the first byte of these names, and
the correct way to spell it would become:

apply.c:            SWAP(&p->new_name, &p->old_name);

The latter clearly looks like swapping the new and old names, which
is good, but I do not have any confidence that I will immediately
spot a bug when presented the former under the new world order.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help