[PATCHES v4] Another round of FreeBSD developer build fixes

Uri Simchoni uri at samba.org
Fri Nov 24 22:37:28 UTC 2017

On 11/24/2017 02:09 AM, Andrew Bartlett via samba-technical wrote:
> I don't really like
> [PATCH v4 01/13] s4-cli: increment once in the for loop

What do you have in mind? I agree with the compile that incrementing
both in the for-loop and inside the loop is not very good - people
looking at the code might miss the increment. Shall we transform it into
a while loop?

> I would like some broader comment on:
> [PATCH v4 08/13] build: remove -Wcast-align from developer build
That commit message was already considerably longer than the average
commit message. Following is a broader commit message, please tell if
this is closer to what you had in mind.

    build: remove -Wcast-align from developer build

    -Wcast-align is a warning that warns against casting to a pointer
    with greater alignment. Prior to this patch it was enabled in
    picky developer builds, and this patch disables it.

    On some CPU architectures (e.g. ARM, MIPS), a load/store instruction
    is required to be from an address whose alignment matches the
    load/store size, e.g. a 32-bit load has to access a 32-bit aligned

    Compilers generally emit code that is guaranteed to comply with this
    requirement. As part of keeping that guarantee, each data type has
    an alignment associated with it, where a variable of that type is
    placed at an address which is a multiple of that alignment.

    Now, suppose a pointer to type A, with alignment of 2, is cast to a
    pointer to B, whose alignment is 4. Suddenly we lose the guarantee
    that our pointer is properly aligned, which can cause the program to
    crash on some architectures. The -Wcast-align warning warns against

    That would suggest that -Wcast-align is a good idea.

    However, casting to a pointer of greater alignment is not
    necessarily a bug. The pointer might be known to possess the desired
    alignment. For example:
    1. The original pointer got its value by casting from a greater
       alignment. That is, we cast B* to A* and back to B*. An example
       is casting from sockaddr * to sockaddr_in * - we know the
       sockaddr pointer really points to a sockaddr_in or a

    2. A pointer is known to point to a member of a B structure
       with greater alignment, then calculating the address of the B*
       from the A* is legal, although it involves a cast from A* to
       B* (this BTW has a workaround of casting to void * somewhere
       along the way - see container_of macro in the Linux kernel).

    The Samba code base has some correct code that triggers the
    -Wcast-align warning if enabled. The reason we don't get it all the
    time is that we usually run developer builds with GCC and on
    x86/x86_64 architectures. Those architectures don't have alignment
    requirements, and GCC chooses not to emit a warning in this case.
    That makes the -Wcast-align a no-op on Samba's autobuild.

    OTOH, clang does emit warnings (turned to errors in developer

    So, this warning buys us nothing with x86_64+GCC, and breaks the
    build with clang (and probably on ARM+GCC in developer mode).

    In order to enable this warning properly, we need to map the valid
    uses and disable it there (e.g. there are some C files that
    manipulate sockets, disable the warning on those files. Also have a
    container_of macro that doesn't trigger the warning. Until this is
    done, in order to allow building developer build with clang, this
    warning is not enabled.

    Signed-off-by: Uri Simchoni <uri at samba.org>


More information about the samba-technical mailing list