Re: [PATCH 1/5] add SWAP macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 31, 2017 at 10:03:01PM +0100, René Scharfe wrote:

> > Perhaps we could disallow a side-effect operator in the macro.  By
> > disallow I mean place a comment at the definition to the macro and
> > hopefully catch something like that in code-review.  We have the same
> > issue with the `ALLOC_GROW()` macro.
> 
> SWAP(a++, ...) is caught by the compiler, SWAP(*a++, ...) works fine.
> Technically that should be enough. :)  A comment wouldn't hurt, of course.

One funny thing is that your macro takes address-of itself, behind the
scenes. 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 "&".

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]