Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Doug Evans <dje@google.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: Hui Zhu <hui_zhu@mentor.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] breakpoint remove fail handle bug fix
Date: Thu, 12 Apr 2012 04:45:00 -0000	[thread overview]
Message-ID: <CADPb22Saxu-JLTqn0suqV+5h=RAtD_o_tuEtiKAfi2XXD8fybA@mail.gmail.com> (raw)
In-Reply-To: <20120411203549.GA4715@host2.jankratochvil.net>

On Wed, Apr 11, 2012 at 1:35 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 11 Apr 2012 11:07:08 +0200, Hui Zhu wrote:
>> (gdb) d
>> Delete all breakpoints? (y or n) y
>> warning: Error removing breakpoint 2
>
> I would propose the attached patch instead.
>
> It needs a testcase, would you write one?
>
> Not sure if gdbserver also needs a fix or not.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.
>
>
> Thanks,
> Jan
>
>
> gdb/
> 2012-04-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * linux-nat.c (linux_proc_xfer_partial): Do not check for LEN size and
>        support also WRITEBUF.
>        (linux_xfer_partial): Move here the LEN check from
>        linux_proc_xfer_partial but also call linux_proc_xfer_partial as a last
>        resort if super_xfer_partial fails.

fwiw,
This comment:

+  /* PTRACE_* of super_xfer_partial may not work if the inferior is running.
+     linux_proc_xfer_partial still may work in such case.  */

is not sufficient, to me anyway, to tell me why the code is the way it is.
[E.g., *why* is it important that the write succeed?]
The reader of the code a year from now will have no idea of connection
between this code and breakpoint handling issues.

[Setting aside the fact that recording an operation as having
succeeded when it did not (and even knowing it did not) is just asking
for recurring trouble, and any patch that doesn't fix that underlying
problem is just papering over the real problem.]


  parent reply	other threads:[~2012-04-12  4:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-11  9:32 Hui Zhu
2012-04-11 10:53 ` Yao Qi
2012-04-11 21:33 ` Jan Kratochvil
2012-04-12  4:04   ` Hui Zhu
2012-04-16 10:01     ` Jan Kratochvil
2012-04-16 16:42       ` Hui Zhu
2012-04-17 21:15     ` Jan Kratochvil
2012-04-12  4:45   ` Doug Evans [this message]
2012-04-12  4:35 ` Doug Evans
2012-04-16  9:50   ` Hui Zhu

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='CADPb22Saxu-JLTqn0suqV+5h=RAtD_o_tuEtiKAfi2XXD8fybA@mail.gmail.com' \
    --to=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=hui_zhu@mentor.com \
    --cc=jan.kratochvil@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