[PATCH] Define _POSIX_C_SOURCE if undefined

[PATCH] Define _POSIX_C_SOURCE if undefined

[Khem Raj]

typecast second argument of connect() API to use struct sockaddr*

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 config.c | 3 +++
 mdmon.c  | 2 +-
 msg.c    | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
  * but may not wrap over lines
  *
  */
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
 
 #ifndef CONFFILE
 #define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
 	umask(077); /* ensure no world write access */
-	if (bind(sfd, &addr, sizeof(addr)) < 0) {
+	if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
 
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
-	if (connect(sfd, &addr, sizeof(addr)) < 0) {
+	if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
-- 
2.7.0

[PATCH] Define _POSIX_C_SOURCE if undefined

[Khem Raj]

typecast second argument of connect() API to use struct sockaddr*

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 config.c | 3 +++
 mdmon.c  | 2 +-
 msg.c    | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
  * but may not wrap over lines
  *
  */
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
 
 #ifndef CONFFILE
 #define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
 	umask(077); /* ensure no world write access */
-	if (bind(sfd, &addr, sizeof(addr)) < 0) {
+	if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
 
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
-	if (connect(sfd, &addr, sizeof(addr)) < 0) {
+	if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
-- 
2.7.0

[PATCH] Define _POSIX_C_SOURCE if undefined

[Khem Raj]

typecast second argument of connect() API to use struct sockaddr*

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 config.c | 3 +++
 mdmon.c  | 2 +-
 msg.c    | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
  * but may not wrap over lines
  *
  */
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
 
 #ifndef CONFFILE
 #define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
 	umask(077); /* ensure no world write access */
-	if (bind(sfd, &addr, sizeof(addr)) < 0) {
+	if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
 
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
-	if (connect(sfd, &addr, sizeof(addr)) < 0) {
+	if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
-- 
2.7.0

Re: [PATCH] Define _POSIX_C_SOURCE if undefined

[NeilBrown]

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

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).

 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).

 Please explain.

Thanks,
NeilBrown


> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  config.c | 3 +++
>  mdmon.c  | 2 +-
>  msg.c    | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index c58c8fe..b308b6c 100644
> --- a/config.c
> +++ b/config.c
> @@ -63,6 +63,9 @@
>   * but may not wrap over lines
>   *
>   */
> +#ifndef _POSIX_C_SOURCE
> +#define _POSIX_C_SOURCE 200809L
> +#endif
>  
>  #ifndef CONFFILE
>  #define CONFFILE "/etc/mdadm.conf"
> diff --git a/mdmon.c b/mdmon.c
> index ee12b7c..e4b73d9 100644
> --- a/mdmon.c
> +++ b/mdmon.c
> @@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
>  	addr.sun_family = PF_LOCAL;
>  	strcpy(addr.sun_path, path);
>  	umask(077); /* ensure no world write access */
> -	if (bind(sfd, &addr, sizeof(addr)) < 0) {
> +	if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
>  		close(sfd);
>  		return -1;
>  	}
> diff --git a/msg.c b/msg.c
> index 754630b..45cd450 100644
> --- a/msg.c
> +++ b/msg.c
> @@ -170,7 +170,7 @@ int connect_monitor(char *devname)
>  
>  	addr.sun_family = PF_LOCAL;
>  	strcpy(addr.sun_path, path);
> -	if (connect(sfd, &addr, sizeof(addr)) < 0) {
> +	if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
>  		close(sfd);
>  		return -1;
>  	}
> -- 
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] Define _POSIX_C_SOURCE if undefined

[Khem Raj]

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

Hi NeilBrown

> On Jan 13, 2016, at 4:40 PM, NeilBrown <nfbrown@novell.com> 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.


> 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);


> 
> Please explain.
> 
> Thanks,
> NeilBrown
> 
> 
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>> config.c | 3 +++
>> mdmon.c  | 2 +-
>> msg.c    | 2 +-
>> 3 files changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/config.c b/config.c
>> index c58c8fe..b308b6c 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -63,6 +63,9 @@
>>  * but may not wrap over lines
>>  *
>>  */
>> +#ifndef _POSIX_C_SOURCE
>> +#define _POSIX_C_SOURCE 200809L
>> +#endif
>> 
>> #ifndef CONFFILE
>> #define CONFFILE "/etc/mdadm.conf"
>> diff --git a/mdmon.c b/mdmon.c
>> index ee12b7c..e4b73d9 100644
>> --- a/mdmon.c
>> +++ b/mdmon.c
>> @@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
>> 	addr.sun_family = PF_LOCAL;
>> 	strcpy(addr.sun_path, path);
>> 	umask(077); /* ensure no world write access */
>> -	if (bind(sfd, &addr, sizeof(addr)) < 0) {
>> +	if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
>> 		close(sfd);
>> 		return -1;
>> 	}
>> diff --git a/msg.c b/msg.c
>> index 754630b..45cd450 100644
>> --- a/msg.c
>> +++ b/msg.c
>> @@ -170,7 +170,7 @@ int connect_monitor(char *devname)
>> 
>> 	addr.sun_family = PF_LOCAL;
>> 	strcpy(addr.sun_path, path);
>> -	if (connect(sfd, &addr, sizeof(addr)) < 0) {
>> +	if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
>> 		close(sfd);
>> 		return -1;
>> 	}
>> --
>> 2.7.0
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 204 bytes --]

Re: [PATCH] Define _POSIX_C_SOURCE if undefined

[NeilBrown]

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

On Thu, Jan 14 2016, Khem Raj wrote:

> Hi NeilBrown
>
>> On Jan 13, 2016, at 4:40 PM, NeilBrown <nfbrown@novell.com> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH] Define _POSIX_C_SOURCE if undefined

[Khem Raj]

typecast second argument of connect() API to use struct sockaddr*

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 config.c | 3 +++
 mdmon.c  | 2 +-
 msg.c    | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index c58c8fe..b308b6c 100644
--- a/config.c
+++ b/config.c
@@ -63,6 +63,9 @@
  * but may not wrap over lines
  *
  */
+#ifndef _POSIX_C_SOURCE
+#define _POSIX_C_SOURCE 200809L
+#endif
 
 #ifndef CONFFILE
 #define CONFFILE "/etc/mdadm.conf"
diff --git a/mdmon.c b/mdmon.c
index ee12b7c..e4b73d9 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -235,7 +235,7 @@ static int make_control_sock(char *devname)
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
 	umask(077); /* ensure no world write access */
-	if (bind(sfd, &addr, sizeof(addr)) < 0) {
+	if (bind(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
diff --git a/msg.c b/msg.c
index 754630b..45cd450 100644
--- a/msg.c
+++ b/msg.c
@@ -170,7 +170,7 @@ int connect_monitor(char *devname)
 
 	addr.sun_family = PF_LOCAL;
 	strcpy(addr.sun_path, path);
-	if (connect(sfd, &addr, sizeof(addr)) < 0) {
+	if (connect(sfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
 		close(sfd);
 		return -1;
 	}
-- 
2.7.0