Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Joel Brobecker <brobecker@adacore.com>
Cc: yao@codesourcery.com, gdb-patches@sourceware.org
Subject: Re: Warnings in native MinGW32 build of GDB 7.8
Date: Wed, 13 Aug 2014 18:02:00 -0000	[thread overview]
Message-ID: <83d2c4i7y2.fsf@gnu.org> (raw)
In-Reply-To: <20140813174227.GF4881@adacore.com>

> Date: Wed, 13 Aug 2014 10:42:27 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
> 
> > > >  #include "defs.h"
> > > > +#include "target.h"	/* for 'enum target_xfer_status' */
> > > 
> > > I can see enum target_xfer_status is used in this c file.  This is good
> > > to me.
> > 
> > Not sure what you mean here.  Do you agree with this change?  If not,
> > why not?
> 
> I think Yao was agreeing with the patch. I agree with the change
> as well, so you can commit that part right away.
> 
> > > >  /* Dynamic target-system-dependent parameters for GDB.  */
> > > > +#include "frame.h"	/* for 'struct frame_id' */
> > > 
> > > It is unclear to me why do we need this include?
> > 
> > Because 'struct frame_id' is otherwise not defined, and I get warnings
> > like this one:
> > 
> >      In file included from defs.h:631,
> > 		      from gdb.c:19:
> >      gdbarch.h:429: warning: parameter has incomplete type
> >      gdbarch.h:430: warning: parameter has incomplete type
> 
> I think we should fix gdbarch.h to include frame.h instead, which
> effectively means adjusting gdbarch.sh. The solution you chose seems
> to be relying on an indirect include, which we really really try
> to avoid. Yao's question is a good example of one of the reasons
> why we avoid that; but there is also the fact that not everyone
> will need frame.h's declarations.
> 
> > > > --- gdb/target-dcache.c~0	2014-06-11 19:34:41.000000000 +0300
> > > > +++ gdb/target-dcache.c	2014-08-09 16:17:42.244875000 +0300
> > > > @@ -16,6 +16,7 @@
> > > >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > > >  
> > > >  #include "defs.h"
> > > > +#include "target.h"	/* for 'enum target_xfer_status' */
> > > 
> > > enum target_xfer_status isn't used in target-dcache.c.  Do we really
> > > need this?
> > 
> > It is used in dcache.h which target-dcache.c includes:
> > 
> >      In file included from target-dcache.h:21,
> > 		      from target-dcache.c:19:
> >      dcache.h:42: warning: parameter has incomplete type
> 
> Same here, dcache.h should be the one including target.h.

I can do the 1st and the 3rd parts, but I'd prefer not to touch
gdbarch.sh.  Could one of you please do that?

TIA


  reply	other threads:[~2014-08-13 18:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-09 14:10 Eli Zaretskii
2014-08-13  4:11 ` Yao Qi
2014-08-13 15:21   ` Eli Zaretskii
2014-08-13 17:42     ` Joel Brobecker
2014-08-13 18:02       ` Eli Zaretskii [this message]
2014-08-13 18:10         ` Joel Brobecker
2014-08-13 18:26           ` Joel Brobecker
2014-08-13 18:48             ` Eli Zaretskii
2014-08-15 12:33               ` Joel Brobecker
2014-08-15 14:22                 ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83d2c4i7y2.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=yao@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox