RE: [PATCH] gdb/testsuite: Updated includes

[Lukas Durfina <ldurfina at tachyum dot com>]

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