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 <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Mirsad Todorovac <[email protected]>
---
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>
--
2.34.1
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/[email protected]/
>
> 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: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/[email protected]/
>
> 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/[email protected]/
> >
> > 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 <[email protected]> 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@googlecom/
> > >
> > > 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