On Thu, Jan 14 2016, Khem Raj wrote:
> Hi NeilBrown
>
>> On Jan 13, 2016, at 4:40 PM, NeilBrown <nfbrown@xxxxxxxxxx> wrote:
>>
>> On Wed, Jan 13 2016, Khem Raj wrote:
>>
>>> typecast second argument of connect() API to use struct sockaddr*
>>>
>>
>> Hi,
>> You have told us what this patch does, but not why anyone should care.
>> Just a sentence or two is probably enough. Are you getting compiler
>> warnings (if so, what are they). Are we violating some standard (which
>> one).
>>
>
> No there is no violation of standard. It helps to port it to work with musl
> There is code in config.c which
> is conditionalized like
>
>
> #if _XOPEN_SOURCE >= 700 || _POSIX_C_SOURCE >= 200809L
> …
> #endif
>
> but we do not define _POSIX_C_SOURCE, glibc defines it in features.h so
> it gets in implicitly with glibc however when we use another libc
> implementation e.g. musl this define is not defined in libc and open
> group documentation says application should ensure that the feature test
> macro _POSIX_C_SOURCE is defined. So this adds a fallback and lets it
> port to musl.
Excellent - thanks.
So the first patch would just have the conditional #define, would have
the same subject as your original patch, and would say something like:
config.c uses _POSIX_C_SOURCE which is defined in features.h when glibc
is used, but isn't defined when musl is used. So provide a reasonable
default.
>
>
>> Is there a connection between defining _POSIX_C_SOURCE (as described in
>> the subject) and the second argument to connect (as mentioned in the
>> comment above) and the second argument to bind (as not mentioned until
>> the code).
>
> No, they are not connected. This is giving compiler diagnostics about type
> mismatches when using musl
> since definitions of sockaddr_un and sockaddr are different.
>
> musl defines the connect signature as
>
> int connect (int, const struct sockaddr *, socklen_t);
>
> which is inline with open group specs.
>
> It doesnt warn with glibc
> because the signature of connect() uses a union of struct types for second
> argument which is a GNU extention. Here are some part from
> /usr/include/sys/socket.h
>
>
> # define __SOCKADDR_ALLTYPES \
> __SOCKADDR_ONETYPE (sockaddr) \
> __SOCKADDR_ONETYPE (sockaddr_at) \
> __SOCKADDR_ONETYPE (sockaddr_ax25) \
> __SOCKADDR_ONETYPE (sockaddr_dl) \
> __SOCKADDR_ONETYPE (sockaddr_eon) \
> __SOCKADDR_ONETYPE (sockaddr_in) \
> __SOCKADDR_ONETYPE (sockaddr_in6) \
> __SOCKADDR_ONETYPE (sockaddr_inarp) \
> __SOCKADDR_ONETYPE (sockaddr_ipx) \
> __SOCKADDR_ONETYPE (sockaddr_iso) \
> __SOCKADDR_ONETYPE (sockaddr_ns) \
> __SOCKADDR_ONETYPE (sockaddr_un) \
> __SOCKADDR_ONETYPE (sockaddr_x25)
>
> typedef union { __SOCKADDR_ALLTYPES
> } __CONST_SOCKADDR_ARG __attribute__ ((__transparent_union__));
>
>
> extern int connect (int __fd, __CONST_SOCKADDR_ARG __addr, socklen_t __len);
>
Ok, so adding the casts should happen in a second patch with a subject
like
Subject: add casts for the addr arg of connect and bind
and then the comment above the patch would say something like:
glibc allows the addr arg to connect and socket to be any of a number
of 'sockaddr_*' types, but musl requires 'const struct sockaddr *'
which is in line with open group specs. So add casts to allow
compilation with musl.
If you could resend as two separate patches with appropriate
descriptions (use the above or alter to suit your preference) then it
will be obvious what the purpose of the patches is, and I'll apply them.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
