| Message ID | 656c3b4a-0481-4634-9dd4-19bb9e4cd612@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/1] selftests/sgx: Fix the implicit declaration of asprintf() compiler error | expand |
On 5/10/24 1:37 PM, Mirsad Todorovac wrote: ... > The fix defines __USE_GNU before including <stdio.h> in case it isn't already > defined. After this intervention the module compiles OK. Instead of interventions, I believe the standard way to do this is to simply define _GNU_SOURCE before including the header file(s). For example, the following also fixes the compilation failure on Ubuntu: diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9820b3809c69..bb6e795d06e2 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright(c) 2016-20 Intel Corporation. */ +#define _GNU_SOURCE #include <cpuid.h> #include <elf.h> #include <errno.h> However, that's not required, because Edward Liaw is already on v4 of a patchset[1] that fixes up the _GNU_SOURCE problem for all selftests. [1] https://lore.kernel.org/all/20240510000842.410729-2-edliaw@google.com/ thanks,
On 5/10/24 22:52, John Hubbard wrote: > On 5/10/24 1:37 PM, Mirsad Todorovac wrote: > ... >> The fix defines __USE_GNU before including <stdio.h> in case it isn't already >> defined. After this intervention the module compiles OK. > > Instead of interventions, I believe the standard way to do this is to simply > define _GNU_SOURCE before including the header file(s). For example, the > following also fixes the compilation failure on Ubuntu: > > diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c > index 9820b3809c69..bb6e795d06e2 100644 > --- a/tools/testing/selftests/sgx/main.c > +++ b/tools/testing/selftests/sgx/main.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* Copyright(c) 2016-20 Intel Corporation. */ > > +#define _GNU_SOURCE > #include <cpuid.h> > #include <elf.h> > #include <errno.h> > > > However, that's not required, because Edward Liaw is already on v4 of > a patchset[1] that fixes up the _GNU_SOURCE problem for all selftests. > > [1] https://lore.kernel.org/all/20240510000842.410729-2-edliaw@google.com/ > > thanks, Hi, Yes, I actually like Ed's solution more, because it solves the asprintf() prototype problem with TEST_HARNESS_MAIN macro for all of the tests. Sorry for the noise and the time wasted reviewing. 8-| Best regards, Mirsad Todorovac
On Fri May 10, 2024 at 11:37 PM EEST, Mirsad Todorovac wrote: > tools/testing/selftests/sgx/main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c > index 9820b3809c69..f5cb426bd797 100644 > --- a/tools/testing/selftests/sgx/main.c > +++ b/tools/testing/selftests/sgx/main.c > @@ -6,6 +6,9 @@ > #include <errno.h> > #include <fcntl.h> > #include <stdbool.h> > +#ifndef __USE_GNU > +#define __USE_GNU > +#endif > #include <stdio.h> > #include <stdint.h> > #include <stdlib.h> __USE_GNU is an internal define, never use it for anything. Use #define _GNU_SOURCE instead without ifndef/endif [1]. [1] https://man7.org/linux/man-pages/man3/asprintf.3.html BR, Jarkko
On Fri May 10, 2024 at 11:52 PM EEST, John Hubbard wrote: > On 5/10/24 1:37 PM, Mirsad Todorovac wrote: > ... > > The fix defines __USE_GNU before including <stdio.h> in case it isn't already > > defined. After this intervention the module compiles OK. > > Instead of interventions, I believe the standard way to do this is to simply > define _GNU_SOURCE before including the header file(s). For example, the > following also fixes the compilation failure on Ubuntu: > > diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c > index 9820b3809c69..bb6e795d06e2 100644 > --- a/tools/testing/selftests/sgx/main.c > +++ b/tools/testing/selftests/sgx/main.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* Copyright(c) 2016-20 Intel Corporation. */ > > +#define _GNU_SOURCE > #include <cpuid.h> > #include <elf.h> > #include <errno.h> > > > However, that's not required, because Edward Liaw is already on v4 of > a patchset[1] that fixes up the _GNU_SOURCE problem for all selftests. > > [1] https://lore.kernel.org/all/20240510000842.410729-2-edliaw@google.com/ > > thanks, Yeah this and also Link: https://man7.org/linux/man-pages/man3/asprintf.3.html And those crazy dumps could go away. The code does not use per man page aprintf correctly and that is exactly the rationale. It is enough then to just tell that not having _GNU_SOURCE results a compilation error. BR, Jarkko
On Sat May 11, 2024 at 12:02 AM EEST, Mirsad Todorovac wrote: > On 5/10/24 22:52, John Hubbard wrote: > > On 5/10/24 1:37 PM, Mirsad Todorovac wrote: > > ... > >> The fix defines __USE_GNU before including <stdio.h> in case it isn't already > >> defined. After this intervention the module compiles OK. > > > > Instead of interventions, I believe the standard way to do this is to simply > > define _GNU_SOURCE before including the header file(s). For example, the > > following also fixes the compilation failure on Ubuntu: > > > > diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c > > index 9820b3809c69..bb6e795d06e2 100644 > > --- a/tools/testing/selftests/sgx/main.c > > +++ b/tools/testing/selftests/sgx/main.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* Copyright(c) 2016-20 Intel Corporation. */ > > > > +#define _GNU_SOURCE > > #include <cpuid.h> > > #include <elf.h> > > #include <errno.h> > > > > > > However, that's not required, because Edward Liaw is already on v4 of > > a patchset[1] that fixes up the _GNU_SOURCE problem for all selftests. > > > > [1] https://lore.kernel.org/all/20240510000842.410729-2-edliaw@google.com/ > > > > thanks, > > Hi, > > Yes, I actually like Ed's solution more, because it solves the asprintf() prototype > problem with TEST_HARNESS_MAIN macro for all of the tests. > > Sorry for the noise and the time wasted reviewing. 8-| > > Best regards, > Mirsad Todorovac Yeah, well, it does not cause any harm and I was not sure when the patch set is in mainline so thus gave the pointers. Anyway, never ever touch __USE_GNU and always look at the man page from man7.org next time and should cause less friction... BR, Jarkko
Thanks for your explanation. I did not realise that __USE_GNU is evil. :-/ FWIW, there is a sound explanation of the difference between _GNU_SOURCE and __USE_GNU here: https://stackoverflow.com/questions/7296963/gnu-source-and-use-gnu Thanks, Mirsad On Mon, May 13, 2024 at 1:02 AM Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Sat May 11, 2024 at 12:02 AM EEST, Mirsad Todorovac wrote: > > On 5/10/24 22:52, John Hubbard wrote: > > > On 5/10/24 1:37 PM, Mirsad Todorovac wrote: > > > ... > > >> The fix defines __USE_GNU before including <stdio.h> in case it isn't already > > >> defined. After this intervention the module compiles OK. > > > > > > Instead of interventions, I believe the standard way to do this is to simply > > > define _GNU_SOURCE before including the header file(s). For example, the > > > following also fixes the compilation failure on Ubuntu: > > > > > > diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c > > > index 9820b3809c69..bb6e795d06e2 100644 > > > --- a/tools/testing/selftests/sgx/main.c > > > +++ b/tools/testing/selftests/sgx/main.c > > > @@ -1,6 +1,7 @@ > > > // SPDX-License-Identifier: GPL-2.0 > > > /* Copyright(c) 2016-20 Intel Corporation. */ > > > > > > +#define _GNU_SOURCE > > > #include <cpuid.h> > > > #include <elf.h> > > > #include <errno.h> > > > > > > > > > However, that's not required, because Edward Liaw is already on v4 of > > > a patchset[1] that fixes up the _GNU_SOURCE problem for all selftests. > > > > > > [1] https://lore.kernel.org/all/20240510000842.410729-2-edliaw@google.com/ > > > > > > thanks, > > > > Hi, > > > > Yes, I actually like Ed's solution more, because it solves the asprintf() prototype > > problem with TEST_HARNESS_MAIN macro for all of the tests. > > > > Sorry for the noise and the time wasted reviewing. 8-| > > > > Best regards, > > Mirsad Todorovac > > Yeah, well, it does not cause any harm and I was not sure when the patch > set is in mainline so thus gave the pointers. Anyway, never ever touch > __USE_GNU and always look at the man page from man7.org next time and > should cause less friction... > > BR, Jarkko
On Mon May 13, 2024 at 12:43 PM EEST, Mirsad Todorovac wrote: > Thanks for your explanation. > > I did not realise that __USE_GNU is evil. :-/ It's not "evil" IMHO. It is not just part of defined API :-) Thus the official man pages are your friend. > > FWIW, there is a sound explanation of the difference between > _GNU_SOURCE and __USE_GNU > here: https://stackoverflow.com/questions/7296963/gnu-source-and-use-gnu > > Thanks, > Mirsad BR, Jarkko
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9820b3809c69..f5cb426bd797 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -6,6 +6,9 @@ #include <errno.h> #include <fcntl.h> #include <stdbool.h> +#ifndef __USE_GNU +#define __USE_GNU +#endif #include <stdio.h> #include <stdint.h> #include <stdlib.h>
The selftest/sgx/main.c didn't compile with [-Werror=implicit-function-declaration] [edited]: make[3]: Entering directory 'tools/testing/selftests/sgx' gcc -Wall -Werror -g -Itools/testing/selftests/../../../tools/include -fPIC -c main.c \ -o tools/testing/selftests/sgx/main.o In file included from main.c:21: ../kselftest_harness.h: In function ‘__run_test’: ../kselftest_harness.h:1169:13: error: implicit declaration of function ‘asprintf’; \ did you mean ‘vsprintf’? [-Werror=implicit-function-declaration] 1169 | if (asprintf(&test_name, "%s%s%s.%s", f->name, | ^~~~~~~~ | vsprintf cc1: all warnings being treated as errors make[3]: *** [Makefile:36: tools/testing/selftests/sgx/main.o] Error 1 The cause is in the included <stdio.h> on Ubuntu 22.04 LTS: 19 /* 20 * ISO C99 Standard: 7.19 Input/output <stdio.h> 21 */ . . . 387 #if __GLIBC_USE (LIB_EXT2) 388 /* Write formatted output to a string dynamically allocated with `malloc'. 389 Store the address of the string in *PTR. */ 390 extern int vasprintf (char **__restrict __ptr, const char *__restrict __f, 391 __gnuc_va_list __arg) 392 __THROWNL __attribute__ ((__format__ (__printf__, 2, 0))) __wur; 393 extern int __asprintf (char **__restrict __ptr, 394 const char *__restrict __fmt, ...) 395 __THROWNL __attribute__ ((__format__ (__printf__, 2, 3))) __wur; 396 extern int asprintf (char **__restrict __ptr, 397 const char *__restrict __fmt, ...) 398 __THROWNL __attribute__ ((__format__ (__printf__, 2, 3))) __wur; 399 #endif __GLIBC_USE (LIB_EXT2) expands into __GLIBC_USE_LIB_EXT2 as defined here: /usr/include/features.h:186:#define __GLIBC_USE(F) __GLIBC_USE_ ## F Now, what is unobvious is that <stdio.h> includes /usr/include/x86_64-linux-gnu/bits/libc-header-start.h: ------------------------------------------------------ 35 /* ISO/IEC TR 24731-2:2010 defines the __STDC_WANT_LIB_EXT2__ 36 macro. */ 37 #undef __GLIBC_USE_LIB_EXT2 38 #if (defined __USE_GNU \ 39 || (defined __STDC_WANT_LIB_EXT2__ && __STDC_WANT_LIB_EXT2__ > 0)) 40 # define __GLIBC_USE_LIB_EXT2 1 41 #else 42 # define __GLIBC_USE_LIB_EXT2 0 43 #endif This makes <stdio.h> exclude line 396 and asprintf() prototype from normal include file processing. The fix defines __USE_GNU before including <stdio.h> in case it isn't already defined. After this intervention the module compiles OK. Converting snprintf() to asprintf() in selftests/kselftest_harness.h:1169 created this new dependency and the implicit declaration broke the compilation. Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX") Cc: Edward Liaw <edliaw@google.com> Cc: Jarkko Sakkinen <jarkko@kernel.org> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: Shuah Khan <shuah@kernel.org> Cc: linux-sgx@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Mirsad Todorovac <mtodorov69@gmail.com> --- tools/testing/selftests/sgx/main.c | 3 +++ 1 file changed, 3 insertions(+)