diff mbox series

[v1,1/1] selftests/sgx: Fix the implicit declaration of asprintf() compiler error

Message ID 656c3b4a-0481-4634-9dd4-19bb9e4cd612@gmail.com
State New
Headers show
Series [v1,1/1] selftests/sgx: Fix the implicit declaration of asprintf() compiler error | expand

Commit Message

Mirsad Todorovac May 10, 2024, 8:37 p.m. UTC
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(+)

Comments

Jarkko Sakkinen May 12, 2024, 10:48 p.m. UTC | #1
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
Jarkko Sakkinen May 12, 2024, 11:02 p.m. UTC | #2
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
Jarkko Sakkinen May 13, 2024, 11:20 a.m. UTC | #3
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 mbox series

Patch

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>