Re: [PATCH 1/5] add SWAP macro

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

Hi Hannes,

On Mon, 30 Jan 2017, Johannes Sixt wrote:

> Am 30.01.2017 um 17:01 schrieb Johannes
> Schindelin:
> > On Sat, 28 Jan 2017, René Scharfe wrote:
> > > diff --git a/git-compat-util.h
> > > b/git-compat-util.h
> > > index 87237b092b..66cd466eea 100644
> > > --- a/git-compat-util.h
> > > +++ b/git-compat-util.h
> > > @@ -527,6 +527,16 @@ static inline int
> > > @@ ends_with(const char *str, const char
> > > @@ *suffix)
> > >  	return strip_suffix(str, suffix, &len);
> > >  }
> > >
> > > +#define SWAP(a, b) do {
> > > \
> > > +	void *_swap_a_ptr = &(a);
> > > \
> > > +	void *_swap_b_ptr = &(b);
> > > \
> > > +	unsigned char _swap_buffer[sizeof(a)];
> > > \
> > > +	memcpy(_swap_buffer, _swap_a_ptr,
> > > sizeof(a));		\
> > > +	memcpy(_swap_a_ptr, _swap_b_ptr,
> > > sizeof(a) +		\
> > > +	       BUILD_ASSERT_OR_ZERO(sizeof(a)
> > > == sizeof(b)));	\
> > > +	memcpy(_swap_b_ptr, _swap_buffer,
> > > sizeof(a));		\
> > > +} while (0)
> > > +
> > >  #if defined(NO_MMAP) ||
> > >  defined(USE_WIN32_MMAP)
> >
> > It may seem as a matter of taste, or maybe
> > not: I prefer this without the
> > _swap_a_ptr
> 
> The purpose of these pointers is certainly to
> avoid that the macro arguments are evaluated
> more than once.

I mistook "a" being used in sizeof(a) for breaking that assumption, but of
course a is *not* evaluated in that case. It is curious, though, that an
expression like "sizeof(a++)" would not be rejected.

Further, what would SWAP(a++, b) do? Swap a and b, and *then* increment a?

Ciao,
Johannes