Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Hans-Peter Nilsson <hp@bitrange.com>
To: Mike Frysinger <vapier@gentoo.org>
Cc: gdb-patches@sourceware.org
Subject: Re: Finalizing fix committed for "[PATCH] sim: make sure to include strsignal prototype"
Date: Sat, 24 Mar 2012 20:55:00 -0000	[thread overview]
Message-ID: <alpine.BSF.2.00.1203241615470.67956@dair.pair.com> (raw)
In-Reply-To: <201203241424.16156.vapier@gentoo.org>

On Sat, 24 Mar 2012, Mike Frysinger wrote:

> On Saturday 24 March 2012 06:55:28 Hans-Peter Nilsson wrote:
> > This patch was missing the critical include of cconfig.h so you
> > should still have seen warnings about implicit declarations of
> > strsignal when testing.
>
> as i mentioned earlier, there are no warnings for me because my glibc conforms
> to the 2008 POSIX spec

I saw that, but that didn't apply: the include of string.h in
nrun.c after your patch was guarded by a macro that wasn't
defined, with no other include of string.h *when compiling
nrun.c for the CRIS sim* to which you were alerted.  Right, you
found there was another via sim-main.h or something for the MIPS
compilation, so just recompiling that wouldn't test anything.
Without describing how the patch was tested I assumed you did
so for cris-elf.  When compiling with a new enough glibc for
string.h-without-_GNU_SOURCE, but gcc that I think is older than
what you used (4.3.0, JFTR re the warnings) I got in my
build-log:
/home/hp/sim/src/sim/cris/../common/nrun.c: In function 'main':
/home/hp/sim/src/sim/cris/../common/nrun.c:216: warning: implicit declaration of function 'strsignal'
/home/hp/sim/src/sim/cris/../common/nrun.c:216: warning: format '%s' expects type 'char *', but argument 4 has type 'int'

> > assembly-code tests and also apparently a compiler is not
> > optional as for cris-sim, but required, causing spurious errors.
>
> if i knew how to make it optional, i would have them auto-skipped ;)

See top of sim/testsuite/sim/cris/c/c.exp.  Care taken to still
iterate over the C test-cases so the "untested" count gets right.

> > FWIW, all sim tests fail linking for cr16-elf+cr16-sim, maybe my
> > locally added baseboard is out of date.
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=12385

That's what I saw, good.

> > --- nrun.c	24 Mar 2012 05:38:35 -0000	1.17
> > +++ nrun.c	24 Mar 2012 09:31:09 -0000	1.18
> >
> > +/* Need to be before general includes, to pick up e.g. _GNU_SOURCE.  */
> > +#ifdef HAVE_CONFIG_H
> > +#include "cconfig.h"
> > +#include "tconfig.h"
> > +#endif
>
> the cconfig.h makes sense, but i don't think the tconfig.h does

This is just a copy of the construct from run.c after checking
that there indeed was a tconfig.h generated; a symlink.  Never
looked into the why of it, feel free investigate whether it's
old cruft used nowhere.

brgds, H-P
PS. thanks for the ld report!


  reply	other threads:[~2012-03-24 20:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-23  4:23 [PATCH] sim: make sure to include strsignal prototype Mike Frysinger
2012-03-23  4:35 ` Mike Frysinger
2012-03-23  5:24 ` Hans-Peter Nilsson
2012-03-23 15:43   ` Mike Frysinger
2012-03-23 22:12     ` Hans-Peter Nilsson
2012-03-23 22:43       ` Mike Frysinger
2012-03-24  5:34     ` Mike Frysinger
2012-03-24  4:12   ` Mike Frysinger
2012-03-24  5:53     ` Hans-Peter Nilsson
2012-03-24  6:12       ` Mike Frysinger
2012-03-24  6:49         ` Hans-Peter Nilsson
2012-03-24 18:18           ` Mike Frysinger
2012-03-24 10:55   ` Finalizing fix committed for "[PATCH] sim: make sure to include strsignal prototype" Hans-Peter Nilsson
2012-03-24 18:24     ` Mike Frysinger
2012-03-24 20:55       ` Hans-Peter Nilsson [this message]
2012-03-25  8:14         ` Mike Frysinger
2012-03-25 14:03           ` Hans-Peter Nilsson
2012-03-25  7:40       ` Mike Frysinger
2012-03-25  7:54         ` Mike Frysinger
2012-03-25 14:26           ` Hans-Peter Nilsson
2012-03-24  5:39 ` [PATCH] sim: make sure to include strsignal prototype Mike Frysinger

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=alpine.BSF.2.00.1203241615470.67956@dair.pair.com \
    --to=hp@bitrange.com \
    --cc=gdb-patches@sourceware.org \
    --cc=vapier@gentoo.org \
    /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