No objection from me, but Ian is the maintainer of libiberty, so I'll defer
to him, especially about style and overall software engineering.
The C23 change presumably will break on Alpha OSF/1 as well. Does GCC
still support OSF/1? It might be preferred to delete the block entirely
instead of #ifndef _AIX.
Thanks, David
On Fri, Dec 6, 2024 at 7:20 AM Sam James <s...@gentoo.org> wrote:
> swamy sangamesh <swamy.sangam...@gmail.com> writes:
>
> > Dear Community,
> >
> > Please let me know if the attached patch is fine.
>
> For such patches, I recommend CCing the maintainers of relevant
> components. In this case, that's David Edelsohn, being the AIX
> maintainer (done it for you here).
>
> I can't approve it but I imagine the patch is fine given GCC dropped
> support for such old AIX a long time ago.
>
> >
> > Thanks,
> > Sangamesh
> >
> > On Tue, Dec 3, 2024 at 11:19 PM swamy sangamesh <
> swamy.sangam...@gmail.com> wrote:
> >
> > Hi Eric,
> >
> > Thanks for the review.
> >
> > I too think removing the define is a better approach and seems these
> won't be needed.
> > From the comment it looks like that these were added long back and
> conflicting declarations were their until C23
> > standard uncovered it.
> >
> > If removing define is fine then i can send a final patch.
> >
> > Thanks,
> > Sangamesh
> >
> > On Tue, Dec 3, 2024 at 9:11 AM Eric Gallager <eg...@gwmail.gwu.edu>
> wrote:
> >
> > On Mon, Dec 2, 2024 at 1:01 PM swamy sangamesh
> > <swamy.sangam...@gmail.com> wrote:
> > >
> > > Dear Community,
> > >
> > > Please let me know your comment.
> > > Or is it more appropriate to have changes with header guard like this
> ?
> > >
> >
> > I personally think it's better to just remove the define, but if
> > you're going to leave it in and guard it with a macro instead, I'd use
> > something a bit more specific than just "_AIX".
> >
> > > --- a/libiberty/getopt.c
> > > +++ b/libiberty/getopt.c
> > > @@ -25,9 +25,11 @@
> > > ^L
> > > /* This tells Alpha OSF/1 not to define a getopt prototype in
> <stdio.h>.
> > > Ditto for AIX 3.2 and <stdlib.h>. */
> > > +#ifndef _AIX
> > > #ifndef _NO_PROTO
> > > # define _NO_PROTO
> > > #endif
> > > +#endif
> > >
> > > #ifdef HAVE_CONFIG_H
> > > # include <config.h>
> > >
> > >
> > > Thanks,
> > > Sangamesh
> > >
> > >
> > > On Thu, Nov 28, 2024 at 11:09 AM Sangamesh Mallayya <
> swamy.sangam...@gmail.com> wrote:
> > >>
> > >> libiberty/getopt.c file is defining _NO_PROTO which causes
> conflicting
> > >> declarations for the functions in AIX header files like stdio.h &
> stdlib.h.
> > >> These declarations are being considered as errors in C23 which
> wasn't
> > >> the case with C17.
> > >>
> > >> Here is the error we get.
> > >>
> > >> /gcc_build/./prev-gcc/xgcc -B/gcc_build/./prev-gcc/
> -B/home/sangam/install/GCC/powerpc-ibm-aix7.3.3.0/bin/ -
> > B/home/sangam
> > >> /install/GCC/powerpc-ibm-aix7.3.3.0/bin/
> -B/home/sangam/install/GCC/powerpc-ibm-aix7.3.3.0/lib/ -isystem
> > /home/sangam/ins
> > >> tall/GCC/powerpc-ibm-aix7.3.3.0/include -isystem
> /home/sangam/install/GCC/powerpc-ibm-aix7.3.3.0/sys-include
> > -fno-check
> > >> ing -c -DHAVE_CONFIG_H -g -O2 -fno-checking -I.
> -I/opt/freeware/src/packages/BUILD/gcc/libiberty/../include -
> > W -Wall -W
> > >> write-strings -Wc++-compat -Wstrict-prototypes -Wshadow=local
> -pedantic -D_GNU_SOURCE
> > /opt/freeware/src/packages/BUILD/
> > >> gcc/libiberty/getopt.c -o getopt.o
> > >>
> > >>
> > >> In file included from
> /opt/freeware/src/packages/BUILD/gcc/libiberty/getopt.c:45:
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:593:12: error: conflicting
> types for 'fgetpos64'; have 'int(FILE *,
> > fpos64_t *)
> > >> ' {aka 'int(FILE *, long long int *)'}
> > >> 593 | extern int fgetpos64(FILE *, fpos64_t *);
> > >> | ^~~~~~~~~
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:298:17: note: previous
> declaration of 'fgetpos64' with type 'int
> > (void)'
> > >> 298 | extern int fgetpos();
> > >> | ^~~~~~~
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:594:14: error: conflicting
> types for 'fopen64'; have 'FILE *(const
> > char *, cons
> > >> t char *)'
> > >> 594 | extern FILE *fopen64(const char *, const char *);
> > >> | ^~~~~~~
> > >>
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:259:17: note: previous
> declaration of 'fopen64' with type 'FILE *
> > (void)'
> > >> 259 | extern FILE * fopen();
> > >> | ^~~~~
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:595:14: error: conflicting
> types for 'freopen64'; have 'FILE *(const
> > char *, co
> > >> nst char *, FILE *)'
> > >> 595 | extern FILE *freopen64(const char *, const char *, FILE *);
> > >> | ^~~~~~~~~
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:260:17: note: previous
> declaration of 'freopen64' with type 'FILE *
> > (void)'
> > >> 260 | extern FILE * freopen();
> > >> | ^~~~~~~
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:597:12: error: conflicting
> types for 'fsetpos64'; have 'int(FILE *,
> > const fpos6
> > >> 4_t *)' {aka 'int(FILE *, const long long int *)'}
> > >> 597 | extern int fsetpos64(FILE *, const fpos64_t *);
> > >> | ^~~~~~~~~
> > >> /gcc_build/prev-gcc/include-fixed/stdio.h:300:17: note: previous
> declaration of 'fsetpos64' with type 'int
> > (void)'
> > >> 300 | extern int fsetpos();
> > >> | ^~~~~~~
> > >> In file included from
> /opt/freeware/src/packages/BUILD/gcc/libiberty/getopt.c:216:
> > >> /gcc_build/prev-gcc/include-fixed/stdlib.h: In function 'strtold':
> > >> /gcc_build/prev-gcc/include-fixed/stdlib.h:233:30: error: too many
> arguments to function 'strtod'
> > >>
> > >>
> > >> Compiled with this patch on RHEL8.10 ppc64le as well.
> > >>
> > >> ---
> > >> libiberty/getopt.c | 6 ------
> > >> 1 file changed, 6 deletions(-)
> > >>
> > >> diff --git a/libiberty/getopt.c b/libiberty/getopt.c
> > >> index 2f7086cc0c8..48736d4db41 100644
> > >> --- a/libiberty/getopt.c
> > >> +++ b/libiberty/getopt.c
> > >> @@ -23,12 +23,6 @@
> > >> Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
> 02110-1301,
> > >> USA. */
> > >>
> > >> -/* This tells Alpha OSF/1 not to define a getopt prototype in
> <stdio.h>.
> > >> - Ditto for AIX 3.2 and <stdlib.h>. */
> > >> -#ifndef _NO_PROTO
> > >> -# define _NO_PROTO
> > >> -#endif
> > >> -
> > >> #ifdef HAVE_CONFIG_H
> > >> # include <config.h>
> > >> #endif
> > >> --
> > >> 2.41.0
> > >>
> > >
> > >
> > > --
> > > Thanks & Regards,
> > > Sangamesh
> >
> > --
> > Thanks & Regards,
> > Sangamesh
>
