Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Ricard Wanderlof <ricard.wanderlof@axis.com>
To: Pedro Alves <palves@redhat.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH][CRISv32] Add support for threaded debugging
Date: Tue, 03 Sep 2013 14:58:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1309031653360.10770@lnxricardw.se.axis.com> (raw)
In-Reply-To: <5225F303.4050901@redhat.com>


On Tue, 3 Sep 2013, Pedro Alves wrote:

>>> This part looks OK, though it did raise some eyebrows to have
>>> GNU/Linux-specific code in cris-tdep.c, rather than in a cris-linux-tdep.c
>>> file.  It seems there's no real support for cris bare-metal debugging?
> ...
>> I don't really see the need for it. It doesn't seem like it would be a
>> huge effort (essentially the call to
>> set_gdbarch_fetch_tls_load_module_address and also
>> set_solib_svr4_fetch_link_map_offsets would be put in cris-linux-tdep.c),
>> on the other hand I can't really test that it works as expected,
> ...
> It's mostly about code/design/maintenance sanity.  I won't really mind if the
> split isn't done, but note how the fact that there's a Linux port here
> is being missed often in regular maintenance (because people will look
> for *linux-tdep.*) files.  cris-tdep.c doesn't call linux_init_abi anywhere
> AFAICT, for example, so the cris port lost the adjustment between v1
> and v2 of the gdbarch_gdb_signal_{to,from}_target
> patches, just a few weeks back:
>
> http://sourceware.org/ml/gdb-patches/2013-07/msg00002.html
> http://sourceware.org/ml/gdb-patches/2013-07/msg00651.html
>
> Probably other across-the-board changes have been missed.

That's a good point. I'll see if I find some time to make a rudimentary 
split.

Regarding the specific case above, it's a bit odd though that the CRIS 
port was included in the first patch set but not the second, considering 
they were supplied by the same person. Still, it still makes your point.

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30


  reply	other threads:[~2013-09-03 14:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-30 13:30 [PATCH] Fix CRISv32 compilation Ricard Wanderlof
2013-09-02 18:04 ` [PATCH][CRISv32] Add support for threaded debugging Pedro Alves
2013-09-03 12:35   ` Ricard Wanderlof
2013-09-03 14:32     ` Pedro Alves
2013-09-03 14:58       ` Ricard Wanderlof [this message]
2013-09-03 15:02         ` Pedro Alves

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.DEB.2.00.1309031653360.10770@lnxricardw.se.axis.com \
    --to=ricard.wanderlof@axis.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.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