Re: [PATCH 1/5] add SWAP macro

From: René Scharfe <l.s.r@web.de>
Date: Feb 1, 2017, 18:06 UTC
Message-ID: <bfd0d758-a9c8-9792-6294-9f9ed632cc98@web.de>
In-Reply-To: <20170201114750.r5xdy6emdczmnh4j@sigill.intra.peff.net>

Am 01.02.2017 um 12:47 schrieb Jeff King:

Show 5 quoted lines

> I'm not altogether convinced that SWAP() is an improvement in
> readability. I really like that it's shorter than the code it replaces,
> but there is a danger with introducing opaque constructs. It's one more
> thing for somebody familiar with C but new to the project to learn or
> get wrong.

I'm biased, of course, but SIMPLE_SWAP and SWAP exchange the values of two variables -- how could someone get that wrong? Would it help to name the macro SWAP_VALUES? Or XCHG? ;)

> IMHO the main maintenance gain from René's patch is that you don't have
> to specify the type, which means you can never have a memory-overflow
> bug due to incorrect types. If we lose that, then I don't see all that
> much value in the whole thing.

Size checks could be added to SIMPLE_SWAP as well.

The main benefit of a swap macro is reduced repetition IMHO: Users specify the variables to swap once instead of twice in a special order, and with SWAP they don't need to declare their type again. Squeezing out redundancy makes the code easier to read and modify.

René