| Message ID | 83r40plpp3.fsf@gnu.org |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 28851 invoked by alias); 9 Aug 2014 14:10:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 28838 invoked by uid 89); 9 Aug 2014 14:10:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout20.012.net.il Received: from mtaout20.012.net.il (HELO mtaout20.012.net.il) (80.179.55.166) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 09 Aug 2014 14:09:58 +0000 Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0NA100I00LNPIU00@a-mtaout20.012.net.il> for gdb-patches@sourceware.org; Sat, 09 Aug 2014 17:09:55 +0300 (IDT) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NA100H0CM0IYR80@a-mtaout20.012.net.il> for gdb-patches@sourceware.org; Sat, 09 Aug 2014 17:09:55 +0300 (IDT) Date: Sat, 09 Aug 2014 17:09:44 +0300 From: Eli Zaretskii <eliz@gnu.org> Subject: Warnings in native MinGW32 build of GDB 7.8 To: gdb-patches@sourceware.org Reply-to: Eli Zaretskii <eliz@gnu.org> Message-id: <83r40plpp3.fsf@gnu.org> X-IsSubscribed: yes |
Commit Message
Eli Zaretskii
Aug. 9, 2014, 2:09 p.m. UTC
I've built today a native MinGW32 GDB 7.8, and saw warnings about
incomplete argument types:
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
In file included from target-dcache.h:21,
from target-dcache.c:19:
dcache.h:42: warning: parameter has incomplete type
I fixed that as below, but I wonder why no one else saw this. is this
because I use an ancient version of GCC?
OK to commit the below (master and 7.8 branch), with suitable
ChangeLog entries?
Comments
On 08/09/2014 10:09 PM, Eli Zaretskii wrote: > I fixed that as below, but I wonder why no one else saw this. is this > because I use an ancient version of GCC? I don't see any warning in my mingw32 build. I am using i686-w64-mingw32-gcc 4.8.2. > > OK to commit the below (master and 7.8 branch), with suitable > ChangeLog entries? > > --- gdb/dcache.c~0 2014-07-29 15:37:42.000000000 +0300 > +++ gdb/dcache.c 2014-08-09 16:17:31.823000000 +0300 > @@ -18,6 +18,7 @@ > along with this program. If not, see <http://www.gnu.org/licenses/>. */ > > #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. > #include "dcache.h" > #include "gdbcmd.h" > #include <string.h> > > --- gdb/defs.h~0 2014-07-29 15:37:42.000000000 +0300 > +++ gdb/defs.h 2014-08-09 15:33:59.666750000 +0300 > @@ -628,6 +628,7 @@ > #endif /* alloca not defined */ > > /* 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? > #include "gdbarch.h" > > /* * Maximum size of a register. Something small, but large enough for > > --- 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?
> Date: Wed, 13 Aug 2014 12:07:22 +0800 > From: Yao Qi <yao@codesourcery.com> > > On 08/09/2014 10:09 PM, Eli Zaretskii wrote: > > I fixed that as below, but I wonder why no one else saw this. is this > > because I use an ancient version of GCC? > > I don't see any warning in my mingw32 build. I am using > i686-w64-mingw32-gcc 4.8.2. Your GCC version is eons ahead of mine, and from a different distro on top of that. > > OK to commit the below (master and 7.8 branch), with suitable > > ChangeLog entries? > > > > --- gdb/dcache.c~0 2014-07-29 15:37:42.000000000 +0300 > > +++ gdb/dcache.c 2014-08-09 16:17:31.823000000 +0300 > > @@ -18,6 +18,7 @@ > > along with this program. If not, see <http://www.gnu.org/licenses/>. */ > > > > #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? > > #include "dcache.h" > > #include "gdbcmd.h" > > #include <string.h> > > > > --- gdb/defs.h~0 2014-07-29 15:37:42.000000000 +0300 > > +++ gdb/defs.h 2014-08-09 15:33:59.666750000 +0300 > > @@ -628,6 +628,7 @@ > > #endif /* alloca not defined */ > > > > /* 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 > > --- 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 Thanks for reviewing the patch.
> > > #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.
> 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
> 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? Let me do it now...
--- gdb/dcache.c~0 2014-07-29 15:37:42.000000000 +0300 +++ gdb/dcache.c 2014-08-09 16:17:31.823000000 +0300 @@ -18,6 +18,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include "defs.h" +#include "target.h" /* for 'enum target_xfer_status' */ #include "dcache.h" #include "gdbcmd.h" #include <string.h> --- gdb/defs.h~0 2014-07-29 15:37:42.000000000 +0300 +++ gdb/defs.h 2014-08-09 15:33:59.666750000 +0300 @@ -628,6 +628,7 @@ #endif /* alloca not defined */ /* Dynamic target-system-dependent parameters for GDB. */ +#include "frame.h" /* for 'struct frame_id' */ #include "gdbarch.h" /* * Maximum size of a register. Something small, but large enough for --- 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' */ #include "target-dcache.h" #include "gdbcmd.h" #include "progspace.h"