From: Alan Hayward <Alan.Hayward@arm.com>
To: Gary Benson <gbenson@redhat.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
nd <nd@arm.com>
Subject: Re: [PATCH v2 0/3] Fix stop-on-solib event failures
Date: Mon, 02 Sep 2019 13:20:00 -0000 [thread overview]
Message-ID: <A23173CF-A03F-4AF2-B6EB-9543988344C8@arm.com> (raw)
In-Reply-To: <20190830155151.GA11044@blade.nx>
> On 30 Aug 2019, at 16:51, Gary Benson <gbenson@redhat.com> wrote:
>
> Hi Alan,
>
> Alan Hayward wrote:
>> On some Arm targets (namely the buildbot Arm Docker setup) placing
>> breakpoints on just the solib dynamic probes will cause the target
>> process to not stop. This is due to the probes being invalid - see
>> link in 3/3 for more details.
>>
>> Fix is to fully validate the probes before using the,.
>>
>> Patches 1 and 2 are code refactors. The actual fix is in patch 3.
>
> The code looks good to me, my only caveat being that I think we're
> supposed to wrap lines at 72 columns (unless that changed) and some
> lines in your patches seem too long. I noticed it in comments, but
> possibly it's in code too. With that fixed (or, not fixed if we
> don't wrap at 72 columns any more) I'd say this is good to commit.
> Thank you for doing the work!
>
I’ve always kept within 80. Double checking the coding standards, it has:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Column_limits
I’ve always wondered why there’s a lot of code that wraps earlier and
that explains why. (personally I’d prefer even longer, but that’s just me).
Anyway, pushed patch to head.
Thanks!
Alan.
prev parent reply other threads:[~2019-09-02 13:20 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-21 15:58 Alan Hayward
2019-08-21 15:58 ` [PATCH v2 2/3] Use gdbarch for probe::get_argument_count Alan Hayward
2019-09-04 17:58 ` Sergio Durigan Junior
2019-09-04 18:00 ` Sergio Durigan Junior
2019-08-21 15:58 ` [PATCH v2 1/3] Refactor svr4_create_solib_event_breakpoints Alan Hayward
2019-08-21 15:58 ` [PATCH v2 3/3] Check arguments for all probes before using them Alan Hayward
2019-08-30 15:51 ` [PATCH v2 0/3] Fix stop-on-solib event failures Gary Benson
2019-09-02 13:20 ` Alan Hayward [this message]
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=A23173CF-A03F-4AF2-B6EB-9543988344C8@arm.com \
--to=alan.hayward@arm.com \
--cc=gbenson@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=nd@arm.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).