[PATCH] musl: Add TEMP_FAILURE_RETRY from glibc

[PATCH] musl: Add TEMP_FAILURE_RETRY from glibc

[Adrian Bunk]

Patch it into musl instead of patching all users
(currently elfutils and next ofono).

Signed-off-by: Adrian Bunk <bunk@stusta.de>
---
 ...0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch | 36 +++++++++++++++++++
 meta/recipes-core/musl/musl_git.bb            |  1 +
 ...ternatives-for-glibc-assumptions-hel.patch | 30 +---------------
 3 files changed, 38 insertions(+), 29 deletions(-)
 create mode 100644 meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch

diff --git a/meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch b/meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch
new file mode 100644
index 0000000000..4d48d61651
--- /dev/null
+++ b/meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch
@@ -0,0 +1,36 @@
+From 19cbebc3fa33c6a1a71a6036da4d67c98f859f06 Mon Sep 17 00:00:00 2001
+From: Adrian Bunk <bunk@stusta.de>
+Date: Wed, 15 May 2019 16:08:33 +0300
+Subject: unistd.h: Add TEMP_FAILURE_RETRY
+
+Signed-off-by: Adrian Bunk <bunk@stusta.de>
+Upstream-Status: Inappropriate [oe-specific]
+---
+ include/unistd.h | 11 +++++++++++
+ 1 file changed, 11 insertions(+)
+
+diff --git a/include/unistd.h b/include/unistd.h
+index 9485da7a..8a75af57 100644
+--- a/include/unistd.h
++++ b/include/unistd.h
+@@ -460,6 +460,17 @@ int eaccess(const char *, int);
+ #define _CS_V6_ENV	1148
+ #define _CS_V7_ENV	1149
+ 
++#ifdef _GNU_SOURCE
++#ifndef TEMP_FAILURE_RETRY
++# define TEMP_FAILURE_RETRY(expression) \
++  (__extension__                                                              \
++    ({ long int __result;                                                     \
++       do __result = (long int) (expression);                                 \
++       while (__result == -1L && errno == EINTR);                             \
++       __result; }))
++#endif
++#endif
++
+ #ifdef __cplusplus
+ }
+ #endif
+-- 
+2.20.1
+
diff --git a/meta/recipes-core/musl/musl_git.bb b/meta/recipes-core/musl/musl_git.bb
index 1e324804e1..2b433aed8c 100644
--- a/meta/recipes-core/musl/musl_git.bb
+++ b/meta/recipes-core/musl/musl_git.bb
@@ -15,6 +15,7 @@ PV = "${BASEVER}+git${SRCPV}"
 SRC_URI = "git://git.musl-libc.org/musl \
            file://0001-Make-dynamic-linker-a-relative-symlink-to-libc.patch \
            file://0002-ldso-Use-syslibdir-and-libdir-as-default-pathes-to-l.patch \
+           file://0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch \
           "
 
 S = "${WORKDIR}/git"
diff --git a/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch b/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch
index a4f301d9da..10cdac646c 100644
--- a/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch
+++ b/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch
@@ -64,18 +64,10 @@ index 292082b..308a762 100644
  #include <stddef.h>
  #include <stdint.h>
  #include <sys/param.h>
-@@ -51,6 +51,16 @@
+@@ -51,6 +51,8 @@
  #else
  # error "Unknown byte order"
  #endif
-+#ifndef TEMP_FAILURE_RETRY
-+#define TEMP_FAILURE_RETRY(expression) \
-+  (__extension__							      \
-+    ({ long int __result;						      \
-+       do __result = (long int) (expression);				      \
-+       while (__result == -1L && errno == EINTR);			      \
-+       __result; }))
-+#endif
 +
 +#define error(status, errno, ...) err(status, __VA_ARGS__)
  
@@ -141,26 +133,6 @@ index 56e6105..f4a0649 100644
  #include "../libdw/libdwP.h"	/* DWARF_E_* values are here.  */
  #include "../libelf/libelfP.h"
  #include "system.h"
-diff --git a/libdwfl/libdwfl_crc32_file.c b/libdwfl/libdwfl_crc32_file.c
-index f849128..6f0aca1 100644
---- a/libdwfl/libdwfl_crc32_file.c
-+++ b/libdwfl/libdwfl_crc32_file.c
-@@ -29,6 +29,15 @@
- # include <config.h>
- #endif
- 
-+#ifndef TEMP_FAILURE_RETRY
-+#define TEMP_FAILURE_RETRY(expression) \
-+  (__extension__                                                             \
-+    ({ long int __result;                                                    \
-+       do __result = (long int) (expression);                                \
-+       while (__result == -1L && errno == EINTR);                            \
-+       __result; }))
-+#endif
-+
- #define crc32_file attribute_hidden __libdwfl_crc32_file
- #define crc32 __libdwfl_crc32
- #include <libdwflP.h>
 diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
 index 360e4ee..b5aa397 100644
 --- a/libdwfl/linux-kernel-modules.c
-- 
2.17.1

Re: [PATCH] musl: Add TEMP_FAILURE_RETRY from glibc

[Khem Raj]

On 5/16/19 12:48 AM, Adrian Bunk wrote:
> Patch it into musl instead of patching all users
> (currently elfutils and next ofono).
> 

this violates musl philosophy, and I would like to stay as close as we 
can, so I would suggest that you propose this patch to upstream musl 
first and get an opinion, if it gets accepted, we can change OE

Richard,

Please revert this patch, I know its now in master, but we should wait 
until upstream agrees to apply this, we are trying to extend C library 
which would fall on us eventually forever if upstream does not accept it.

> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> ---
>   ...0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch | 36 +++++++++++++++++++
>   meta/recipes-core/musl/musl_git.bb            |  1 +
>   ...ternatives-for-glibc-assumptions-hel.patch | 30 +---------------
>   3 files changed, 38 insertions(+), 29 deletions(-)
>   create mode 100644 meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch
> 
> diff --git a/meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch b/meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch
> new file mode 100644
> index 0000000000..4d48d61651
> --- /dev/null
> +++ b/meta/recipes-core/musl/musl/0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch
> @@ -0,0 +1,36 @@
> +From 19cbebc3fa33c6a1a71a6036da4d67c98f859f06 Mon Sep 17 00:00:00 2001
> +From: Adrian Bunk <bunk@stusta.de>
> +Date: Wed, 15 May 2019 16:08:33 +0300
> +Subject: unistd.h: Add TEMP_FAILURE_RETRY
> +
> +Signed-off-by: Adrian Bunk <bunk@stusta.de>
> +Upstream-Status: Inappropriate [oe-specific]
> +---
> + include/unistd.h | 11 +++++++++++
> + 1 file changed, 11 insertions(+)
> +
> +diff --git a/include/unistd.h b/include/unistd.h
> +index 9485da7a..8a75af57 100644
> +--- a/include/unistd.h
> ++++ b/include/unistd.h
> +@@ -460,6 +460,17 @@ int eaccess(const char *, int);
> + #define _CS_V6_ENV	1148
> + #define _CS_V7_ENV	1149
> +
> ++#ifdef _GNU_SOURCE
> ++#ifndef TEMP_FAILURE_RETRY
> ++# define TEMP_FAILURE_RETRY(expression) \
> ++  (__extension__                                                              \
> ++    ({ long int __result;                                                     \
> ++       do __result = (long int) (expression);                                 \
> ++       while (__result == -1L && errno == EINTR);                             \
> ++       __result; }))
> ++#endif
> ++#endif
> ++
> + #ifdef __cplusplus
> + }
> + #endif
> +--
> +2.20.1
> +
> diff --git a/meta/recipes-core/musl/musl_git.bb b/meta/recipes-core/musl/musl_git.bb
> index 1e324804e1..2b433aed8c 100644
> --- a/meta/recipes-core/musl/musl_git.bb
> +++ b/meta/recipes-core/musl/musl_git.bb
> @@ -15,6 +15,7 @@ PV = "${BASEVER}+git${SRCPV}"
>   SRC_URI = "git://git.musl-libc.org/musl \
>              file://0001-Make-dynamic-linker-a-relative-symlink-to-libc.patch \
>              file://0002-ldso-Use-syslibdir-and-libdir-as-default-pathes-to-l.patch \
> +           file://0001-unistd.h-Add-TEMP_FAILURE_RETRY.patch \
>             "
>   
>   S = "${WORKDIR}/git"
> diff --git a/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch b/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch
> index a4f301d9da..10cdac646c 100644
> --- a/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch
> +++ b/meta/recipes-devtools/elfutils/files/0008-build-Provide-alternatives-for-glibc-assumptions-hel.patch
> @@ -64,18 +64,10 @@ index 292082b..308a762 100644
>    #include <stddef.h>
>    #include <stdint.h>
>    #include <sys/param.h>
> -@@ -51,6 +51,16 @@
> +@@ -51,6 +51,8 @@
>    #else
>    # error "Unknown byte order"
>    #endif
> -+#ifndef TEMP_FAILURE_RETRY
> -+#define TEMP_FAILURE_RETRY(expression) \
> -+  (__extension__							      \
> -+    ({ long int __result;						      \
> -+       do __result = (long int) (expression);				      \
> -+       while (__result == -1L && errno == EINTR);			      \
> -+       __result; }))
> -+#endif
>   +
>   +#define error(status, errno, ...) err(status, __VA_ARGS__)
>    
> @@ -141,26 +133,6 @@ index 56e6105..f4a0649 100644
>    #include "../libdw/libdwP.h"	/* DWARF_E_* values are here.  */
>    #include "../libelf/libelfP.h"
>    #include "system.h"
> -diff --git a/libdwfl/libdwfl_crc32_file.c b/libdwfl/libdwfl_crc32_file.c
> -index f849128..6f0aca1 100644
> ---- a/libdwfl/libdwfl_crc32_file.c
> -+++ b/libdwfl/libdwfl_crc32_file.c
> -@@ -29,6 +29,15 @@
> - # include <config.h>
> - #endif
> -
> -+#ifndef TEMP_FAILURE_RETRY
> -+#define TEMP_FAILURE_RETRY(expression) \
> -+  (__extension__                                                             \
> -+    ({ long int __result;                                                    \
> -+       do __result = (long int) (expression);                                \
> -+       while (__result == -1L && errno == EINTR);                            \
> -+       __result; }))
> -+#endif
> -+
> - #define crc32_file attribute_hidden __libdwfl_crc32_file
> - #define crc32 __libdwfl_crc32
> - #include <libdwflP.h>
>   diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
>   index 360e4ee..b5aa397 100644
>   --- a/libdwfl/linux-kernel-modules.c
> 

Re: [PATCH] musl: Add TEMP_FAILURE_RETRY from glibc

[Adrian Bunk]

On Mon, May 20, 2019 at 01:19:38PM -0700, Khem Raj wrote:
> 
> 
> On 5/16/19 12:48 AM, Adrian Bunk wrote:
> > Patch it into musl instead of patching all users
> > (currently elfutils and next ofono).
> 
> this violates musl philosophy, and I would like to stay as close as we can,
> so I would suggest that you propose this patch to upstream musl first and
> get an opinion, if it gets accepted, we can change OE

"musl philosophy" is that musl does include the GNU extensions Rich Felker 
likes, and does not include the GNU extensions Rich Felker dislikes.[1]

TEMP_FAILURE_RETRY is in the latter category.

For TEMP_FAILURE_RETRY it's either patching musl once or patching random 
packages all the time, and the patch is not invasive.

> Richard,
> 
> Please revert this patch, I know its now in master, but we should wait until
> upstream agrees to apply this, we are trying to extend C library which would
> fall on us eventually forever if upstream does not accept it.

What is actually the strategy for handling musl changes that will have 
to be shipped in OE forever?

Especially systemd/musl is a combination that will bring an ever-growing 
amount of changes that cannot be upstreamed on either side.

And some of the OE patches to systemd make things compile but are of the
"I wonder if I can get a CVE for that" kind.

The alternative would be to mark systemd as incompatible with musl,
which might be the sustainable thing to do.

cu
Adrian

[1] There seem to be sound technical reasons behind what he likes,
    but this doesn't help when a distribution wants to get existing
    software building with musl.

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] musl: Add TEMP_FAILURE_RETRY from glibc

[Khem Raj]

On 5/20/19 2:16 PM, Adrian Bunk wrote:
> On Mon, May 20, 2019 at 01:19:38PM -0700, Khem Raj wrote:
>>
>>
>> On 5/16/19 12:48 AM, Adrian Bunk wrote:
>>> Patch it into musl instead of patching all users
>>> (currently elfutils and next ofono).
>>
>> this violates musl philosophy, and I would like to stay as close as we can,
>> so I would suggest that you propose this patch to upstream musl first and
>> get an opinion, if it gets accepted, we can change OE
> 
> "musl philosophy" is that musl does include the GNU extensions Rich Felker
> likes, and does not include the GNU extensions Rich Felker dislikes.[1]
> 
> TEMP_FAILURE_RETRY is in the latter category.
> 

I would encourage you to start a discussion. Usually a good reason to 
accept or reject comes out of discussions.

> For TEMP_FAILURE_RETRY it's either patching musl once or patching random
> packages all the time, and the patch is not invasive.
> 

No other musl distro is patching musl especially for this case, we can 
live with that since packages will be patched. Eventually applications 
will wean away from using it.

>> Richard,
>>
>> Please revert this patch, I know its now in master, but we should wait until
>> upstream agrees to apply this, we are trying to extend C library which would
>> fall on us eventually forever if upstream does not accept it.
> 
> What is actually the strategy for handling musl changes that will have
> to be shipped in OE forever?
> 
> Especially systemd/musl is a combination that will bring an ever-growing
> amount of changes that cannot be upstreamed on either side.
> 
> And some of the OE patches to systemd make things compile but are of the
> "I wonder if I can get a CVE for that" kind.
> 
> The alternative would be to mark systemd as incompatible with musl,
> which might be the sustainable thing to do.
> 
> cu
> Adrian
> 
> [1] There seem to be sound technical reasons behind what he likes,
>      but this doesn't help when a distribution wants to get existing
>      software building with musl.
>