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 (mailing list archive)
State New, archived
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

John Hubbard May 10, 2024, 8:52 p.m. UTC | #1
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,
Mirsad Todorovac May 10, 2024, 9:02 p.m. UTC | #2
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
Jarkko Sakkinen May 12, 2024, 10:48 p.m. UTC | #3
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, 10:58 p.m. UTC | #4
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
Jarkko Sakkinen May 12, 2024, 11:02 p.m. UTC | #5
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
Mirsad Todorovac May 13, 2024, 9:43 a.m. UTC | #6
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
Jarkko Sakkinen May 13, 2024, 11:20 a.m. UTC | #7
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>