Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

>> Hello,
>
>> Jan Kara <jack@suse.cz> writes:
>
>> > On Mon 21-11-22 10:33:13, Petr Vorel wrote:
>> >> Hi Jan, all,
>
>> >> > On Thu 17-11-22 16:58:50, Petr Vorel wrote:
>> >> > > Hi Jan, all,
>
>> >> > > > +#define foreach_path(tc, buf, pname) \
>> >> > > > +	for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt,	\
>> >> > > Unfortunately we still support C99 due old compiler on CentOS 7,
>> >> > > therefore int piter needs to be defined outside of for loop.
>
>> >> > Hum, but variable declaration in the for loop is part of C99 standard (as
>> >> > the error message also says). So did you want to say you are compiling
>> >> > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
>> >> > be fully C99 compliant BTW. So what's the situation here?
>> >> I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
>> >> but the default is C90 [1].
>
>> > OK, thanks for explanation.
>
>> >> > That being said I can workaround the problem in the macro, it will just be
>> >> > somewhat uglier. So before doing that I'd like to understand whether
>> >> > following C89 is really required...
>
>> >> I'm don't remember why we have just not specified -std=... already, Cyril had
>> >> some objections, thus Cc him.
>
>> >> Cent0S EOL in 2024-06, we might reconsider to add -std=... to endup this agony
>> >> (errors like this often need to be fixed).
>
>> >> [1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html
>
>> > Given Cyril's reply, should I rework my patch or are we fine with using
>> > C99?
>
>> Well -std=c99 doesn't work, but we can use -std=gnu99. If that doesn't
>> fix it then we should drop centos07 now IMO.
> I'd be ok to put fanotify10: CFLAGS += -std=gnu99 or even CFLAGS += -std=gnu99
> (for all tests) into fanotify's Makefile, which fixes the problem.
> Unless anybody objects, I can change it before merge.

We definitely shouldn't do that. We've had this issue before and it will
come up again. Then we'll have a bunch of tests with this flag added.

>
> @Richie: we need to keep Cent0S 7 working until its EOL in 2024-06.

It appears though that they have not updated the GCC package since 2014.

It looks like glibc and the kernel receive updates. So it's feasible
someone is running LTP on Centos7. However for SLES versions this old we
(statically) compile on a newer release or install a newer GCC package
etc. Or in this case you could even just added '-std=gnu99' to the make
command on Centos7.

So I have to question whether we should support compilation on targets
this old to the extent that we do? I'd rather try to support more
distro's (e.g. buildroot, nixos) or new compilers (e.g. arocc) that
potentially will help with development.

I don't think we should make it impossible to use older distros, but we
should offload some work onto downstream. The expected result of
sticking with older releases is that backports and tweaks are required.

CentOS Stream 8 is on GCC 8 and Stream 9 has GCC 11, why not put that in
CI?

>
> I guess the reason not to specify it in top level Makefile was to have LTP code
> being tested on newer standards. Unless there is a good reason for it, I'd vote
> for putting -std=gnu99 into top level CFLAGS (and increase it after
> CentOS 7 EOL).

You could merge my patch with this one.

>
> Kind regards,
> Petr
>
>> > 								Honza
>
>> >> > > fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
>> >> > >   for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> >> > >   ^
>
>> >> > > fanotify10.c:470:11: error: redefinition of ‘piter’
>> >> > >   for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> >> > >            ^
>> >> > > Kind regards,
>> >> > > Petr


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp