RE: [PATCH] gdb/testsuite: Updated includes

Lukas Durfina
2020-01-07T13:48:00Z

Hi Joel,

Thanks for review.

> -----Original Message-----
> From: Joel Brobecker <brobecker@adacore.com>
> Sent: Sunday, January 5, 2020 7:26 AM
> To: Lukas Durfina <ldurfina@tachyum.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] gdb/testsuite: Updated includes
>
> Hi Lukas,
>
> > Fixed compilation error with musl.
>
> Thanks for the patch!
>
> > ---
> > gdb/testsuite/ChangeLog | 4 ++++
> > gdb/testsuite/gdb.base/fileio.c | 3 +--
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index
> > f7835f35f7..89d403897c 100644
> > --- a/gdb/testsuite/ChangeLog
> > +++ b/gdb/testsuite/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2020-01-02 Lukas Durfina <ldurfina@tachyum.com>
> > +
> > + * gdb.base/fileio.c: Updated includes to be more generic.
>
> For the revision log, I think it's import to be a bit more specific when
> explaining what changes you made and why.
>
> I see you removed the <sys/errno.h> #include without replacing it by a
> #include of <errno.h>. However, this file does make references to errno, so I
> think there is an oversight there.

There is already #include <errno.h> a few lines below, so I skipped it.

>
> Also, I see you not only replace <sys/fcntl.h> by <fcntl.h>, but you also move
> its include further down. Any reason for that.
> Was it to move that include outside of the range of #include-s that include
> files in "sys"?

Exactly. My intention was to move it out of <sys/...> includes. No other reason.

>
> For the ChangeLog, I think it is worth being a little more specific.
> For instance, I would have written the ChangeLog as follow:
>
> * gdb.base/fileio.c: Remove #include of <sys/errno.h>.
> Replace #include of <sys/fcntl.h> by <fcntl.h>.
>
> I'm usually the first in line to take shortcuts with ChangeLog files, but I try to
> be more informative when the cost of being so is so small...

I see your point. Is it fine to send the patch with updated Changelog?

Lukas

>
> > +
> > 2019-12-29 Bernd Edlinger <bernd.edlinger@hotmail.de>
> >
> > * gdb.base/line65535.exp: New file.
> > diff --git a/gdb/testsuite/gdb.base/fileio.c
> > b/gdb/testsuite/gdb.base/fileio.c index 7f482a34d3..0f201518b7 100644
> > --- a/gdb/testsuite/gdb.base/fileio.c
> > +++ b/gdb/testsuite/gdb.base/fileio.c
> > @@ -1,13 +1,12 @@
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > -#include <sys/errno.h>
> > #include <sys/types.h>
> > -#include <sys/fcntl.h>
> > #include <sys/stat.h>
> > #include <sys/time.h>
> > #include <errno.h>
> > #include <sys/wait.h>
> > +#include <fcntl.h>
> > #include <unistd.h>
> > #include <time.h>
> > /* TESTS :
> > --
> > 2.17.1
>
> --
> Joel