Re: [PATCH RFT v3] scripts: define _GNU_SOURCE for all source files

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Khem,

On 27.01.25 21:00, Khem Raj wrote:
> On Mon, Jan 27, 2025 at 11:51 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> musl doesn't define loff_t, unless _GNU_SOURCE is defined, which
>> we don't do everywhere and glibc didn't mind.
>>
>> This led to build errors with musl when code makes it into
>> scripts/ that uses loff_t. This was already fixed once e.g. in commit
>> c0d065fb0aa0 ("scripts: bareboximd: remove usage of loff_t").
>>
>> To fix this issue for good, let's define _GNU_SOURCE globally for all files.
>>
> 
> Since the project is using this define sporadically, it might not be
> concerning but
> It enables GNU-specific extensions and features and if you are
> targeting multiple platforms ( non-linux too )
> then you might want to check those platforms for supporting gnu
> extensions as well.

Good point. AFAICS, _GNU_SOURCE was defined to make asprintf available
and now loff_t. For non-Linux platforms, there's a

  #ifndef __linux__
  typedef long long loff_t;
  #endif

and asprintf was required on nearly all platforms anyways, because the
bundled device tree compiler source makes use of it.

I thus think that this change shouldn't break anything that worked before.
Supporting build on platforms that don't have asprintf might be nice, but
so far nobody complained about that.

Thanks,
Ahmad

> 
>> Link: https://lore.kernel.org/all/CAMKF1sqX7gFCEH78fHS+jkUSieL5crAU_RwAUO-daJL0u+fj0g@mail.gmail.com/
>> Fixes: 5171f4d0696f ("scripts: implement read_fd and pread_full for tools")
>> Suggested-by: Khem Raj <raj.khem@gmail.com>
>> Reported-by: Enrico Jörns <ejo@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> @Enrico, please give this a test. I only tested against glibc.
>>
>> v2 -> v3:
>>   - set _GNU_SOURCE instead of defining loff_t (Khem)
>> v1 -> v2:
>>   - include <linux/types.h> from scripts/common.h (Enrico)
>> v2 was here:
>>   https://lore.barebox.org/barebox/20250127143146.2965544-1-a.fatoum@pengutronix.de/T/#u
>> ---
>>  scripts/Makefile         | 4 ++++
>>  scripts/imx/imx-image.c  | 1 -
>>  scripts/imx/imx.c        | 1 -
>>  scripts/imx9image.c      | 1 -
>>  scripts/kernel-install.c | 1 -
>>  scripts/keytoc.c         | 1 -
>>  scripts/kwbimage.c       | 1 -
>>  7 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/scripts/Makefile b/scripts/Makefile
>> index a780f05bd90f..c1a5b4b74775 100644
>> --- a/scripts/Makefile
>> +++ b/scripts/Makefile
>> @@ -60,6 +60,10 @@ rk-usb-loader-target-userldlibs += `$(CROSS_PKG_CONFIG) --libs libusb-1.0`
>>
>>  userccflags += -I $(srctree)/$(src)/include -isystem $(srctree)/scripts/include
>>
>> +# We need to explicitly set the macro to empty, otherwise it's defined to =1
>> +userccflags += -D_GNU_SOURCE=""
>> +KBUILD_HOSTCFLAGS += -D_GNU_SOURCE=""
>> +
>>  subdir-y                       += mod
>>  subdir-y                       += imx
>>  subdir-$(CONFIG_DTC)           += dtc
>> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
>> index 9ba60dbc5b4c..a8ac34091fc0 100644
>> --- a/scripts/imx/imx-image.c
>> +++ b/scripts/imx/imx-image.c
>> @@ -1,7 +1,6 @@
>>  // SPDX-License-Identifier: GPL-2.0-or-later
>>  // SPDX-FileCopyrightText: 2013 Sascha Hauer, Pengutronix
>>
>> -#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <unistd.h>
>>  #include <getopt.h>
>> diff --git a/scripts/imx/imx.c b/scripts/imx/imx.c
>> index 5ccc116cfe3c..26027f0f3548 100644
>> --- a/scripts/imx/imx.c
>> +++ b/scripts/imx/imx.c
>> @@ -1,7 +1,6 @@
>>  // SPDX-License-Identifier: GPL-2.0-or-later
>>  // SPDX-FileCopyrightText: 2016 Sascha Hauer, Pengutronix
>>
>> -#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <string.h>
>> diff --git a/scripts/imx9image.c b/scripts/imx9image.c
>> index 53fb879882e8..61b7cc943f00 100644
>> --- a/scripts/imx9image.c
>> +++ b/scripts/imx9image.c
>> @@ -8,7 +8,6 @@
>>   *
>>   */
>>
>> -#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <string.h>
>>  #include <stdlib.h>
>> diff --git a/scripts/kernel-install.c b/scripts/kernel-install.c
>> index d943f0288f39..240122f2d224 100644
>> --- a/scripts/kernel-install.c
>> +++ b/scripts/kernel-install.c
>> @@ -72,7 +72,6 @@
>>   *
>>   */
>>
>> -#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <string.h>
>>  #include <stdlib.h>
>> diff --git a/scripts/keytoc.c b/scripts/keytoc.c
>> index c60df8a5f017..2a0d7c415bf0 100644
>> --- a/scripts/keytoc.c
>> +++ b/scripts/keytoc.c
>> @@ -9,7 +9,6 @@
>>   *
>>   */
>>  #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>> -#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <string.h>
>>  #include <time.h>
>> diff --git a/scripts/kwbimage.c b/scripts/kwbimage.c
>> index 370c54c983b5..c58c80e5415f 100644
>> --- a/scripts/kwbimage.c
>> +++ b/scripts/kwbimage.c
>> @@ -30,7 +30,6 @@
>>   * headers in v1 images
>>   */
>>
>> -#define _GNU_SOURCE
>>  #include <stdio.h>
>>  #include <stdint.h>
>>  #include <sys/types.h>
>> --
>> 2.39.5
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |