Re: [PATCH bpf-next v1 7/8] selftests/bpf: Fix using stdout, stderr as struct field names

On Thu, Jul 25, 2024 at 01:27:03PM -0700, Andrii Nakryiko wrote:

On Thu, Jul 25, 2024 at 3:39 AM Tony Ambardar [off-list ref] wrote:

quoted

From: Tony Ambardar <redacted>

Typically stdin, stdout, stderr are treated as reserved identifiers under
ISO/ANSI C, and a libc implementation is free to define these as macros.

Ok, wow that. Do you have a pointer to where in the standard it is
said that stdin/stdout/stderr is some sort of reserved identifier that
can't be used as a field name?

I'll need to dig around to share some references. The short answer IIRC
is there's enough potential variation in their definitions that their
use requires care (or better avoidance).

I really don't like these underscored field names. If we have to
rename, I'd prefer something like env.saved_stdout instead of
env._stdout. But I'd prefer even more if musl wasn't doing this macro
definition, of course...

OK, I'll use clearer names for a v2.

I believe the macro definitions are quite common and old, but "how"
makes a difference: specifically, using parenthesis happens to break our
.stdxxx field names.


In glibc <stdio.h> we have for example:
...
/* Standard streams.  */
extern FILE *stdin;             /* Standard input stream.  */
extern FILE *stdout;            /* Standard output stream.  */
extern FILE *stderr;            /* Standard error output stream.  */
/* C89/C99 say they're macros.  Make them happy.  */
#define stdin stdin
#define stdout stdout
#define stderr stderr
...

while in musl <stdio.h> we have:
...
extern FILE *const stdin;
extern FILE *const stdout;
extern FILE *const stderr;

#define stdin  (stdin)
#define stdout (stdout)
#define stderr (stderr)
...

which borks code in test_progs.c:
...
env.stderr = stderr;
env.stdout = stdout;
...

quoted

This is the case in musl libc and results in compile errors when these
names are reused as struct fields, as with 'struct test_env' and related
usage in test_progs.[ch] and reg_bounds.c.

Rename the fields to _stdout and _stderr to avoid many errors seen building
against musl, e.g.:

  In file included from test_progs.h:6,
                   from test_progs.c:5:
  test_progs.c: In function 'print_test_result':
  test_progs.c:237:21: error: expected identifier before '(' token
    237 |         fprintf(env.stdout, "#%-*d %s:", TEST_NUM_WIDTH, test->test_num, test->test_name);
        |                     ^~~~~~
  test_progs.c:237:9: error: too few arguments to function 'fprintf'
    237 |         fprintf(env.stdout, "#%-*d %s:", TEST_NUM_WIDTH, test->test_num, test->test_name);
        |         ^~~~~~~

Signed-off-by: Tony Ambardar <redacted>
---
 .../selftests/bpf/prog_tests/reg_bounds.c     |  2 +-
 tools/testing/selftests/bpf/test_progs.c      | 66 +++++++++----------
 tools/testing/selftests/bpf/test_progs.h      |  8 +--
 3 files changed, 38 insertions(+), 38 deletions(-)

[...]