public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@codesourcery.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org,
	Joel Brobecker <brobecker@adacore.com>,
	Eli Zaretskii <eliz@gnu.org>,
	Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: [patch 1/9]#2 Rename `enum target_signal' to target_signal_t
Date: Wed, 01 Sep 2010 20:47:00 -0000	[thread overview]
Message-ID: <201009012130.05914.pedro@codesourcery.com> (raw)
In-Reply-To: <20100901201025.GA30493@host1.dyn.jankratochvil.net>

On Wednesday 01 September 2010 21:10:25, Jan Kratochvil wrote:
> On Wed, 01 Sep 2010 21:59:36 +0200, Pedro Alves wrote:
> > Anyway, personally, I'd just do a enum target_signal/enum gdb_signal
> > rename, and stay with that.
> 
> I do not mind about this point, someone can post it.

I thought you were already going to have to rename target_signal_t
anyway for your patch to be acceptible, as it has been raised that
target_signal_t  was not a good name, given that it ends in _t.
Sorry if I misunderstood that.

> Now I thought it is reusable at least as a code cleanup when it is already in
> IMO an acceptable state.

I personally didn't think things looked that cleaner afterwards, but I
do recognize the value in catching bugs at compile time, hence I'll
agree that that justifies code not looking as neat in some cases.  I
was just giving out a suggestion that I thought might be interesting.  I
am really sorry that you go through all this trouble of writing large
mechanical patches that may sometimes be dropped.  But I do believe that you
can save yourself effort sometimes by discussing these changes upfront,
or just post an experimental patch that doesn't fix everything all
uses at once.  That said, once again, I have not objected to your patch.

> I also found as an advantage target_signal_t as a struct could get extended by
> additional information later, as has been done now (although unnecessarily
> - not completely wrongly) for siginfo, addressing the "gdb/signals.h" part:
> 
>                                              However, it is
>    recognized that this set of signals has limitations (such as not
>    distinguishing between various kinds of SIGSEGV, or not
>    distinguishing hitting a breakpoint from finishing a single step).
>    So in the future we may get around this either by adding additional
>    signals for breakpoint, single-step, etc., or by adding signal
>    codes; the latter seems more in the spirit of what BSD, System V,
>    etc. are doing to address these issues.  */

I see no reason that we can't extend target_waitkind/target_waitstatus
instead if we need to.  There's no implying from the above that the
"signal codes" need to be the target_signal itself.  I think that may
be talking about siginfo->si_code even.

-- 
Pedro Alves

  parent reply	other threads:[~2010-09-01 20:30 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-30  7:11 Jan Kratochvil
2010-08-30  8:14 ` Mark Kettenis
2010-08-30  8:24   ` Jan Kratochvil
2010-08-30 10:09     ` Eli Zaretskii
2010-08-30 11:11       ` Mark Kettenis
2010-08-30 14:08         ` Joel Brobecker
2010-08-31 18:28           ` Jan Kratochvil
2010-08-31 18:45             ` Mark Kettenis
2010-09-01  2:03             ` Joel Brobecker
2010-09-01 18:18             ` Joel Brobecker
2010-09-01 18:30               ` Jan Kratochvil
2010-09-01 18:38                 ` Pedro Alves
2010-09-01 18:45                   ` Jan Kratochvil
2010-09-01 18:40                 ` Joel Brobecker
2010-09-01 18:51                   ` Jan Kratochvil
2010-09-01 19:08                     ` Pedro Alves
2010-09-01 19:28                       ` Jan Kratochvil
2010-09-01 20:06                         ` Pedro Alves
2010-09-01 20:06                           ` Daniel Jacobowitz
2010-09-01 20:10                             ` Pedro Alves
2010-09-02  9:46                               ` Pedro Alves
2010-09-02 14:01                                 ` Jan Kratochvil
2010-09-02 15:36                                   ` Joel Brobecker
2010-09-02 19:02                                   ` Pedro Alves
2010-09-02 20:46                                     ` Jan Kratochvil
2010-09-07 18:59                                       ` Jan Kratochvil
2010-09-02 16:02                                 ` Joel Brobecker
2010-09-02 17:04                                   ` Jan Kratochvil
2010-09-06  0:29                                 ` Jan Kratochvil
2010-09-06 13:30                                   ` Pedro Alves
2010-09-06 14:52                                     ` Jan Kratochvil
2010-09-08 23:42                                 ` Jan Kratochvil
2010-09-01 20:23                           ` Jan Kratochvil
2010-09-01 20:30                             ` Mark Kettenis
2010-09-01 20:47                             ` Pedro Alves [this message]
2010-09-01 21:32                         ` Joseph S. Myers
2010-09-01 19:12                     ` Joel Brobecker
2010-09-01 22:37                     ` Tom Tromey
2010-08-30 14:11         ` Eli Zaretskii
2010-08-30 17:34     ` Michael Snyder

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=201009012130.05914.pedro@codesourcery.com \
    --to=pedro@codesourcery.com \
    --cc=brobecker@adacore.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=mark.kettenis@xs4all.nl \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).