Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: Pedro Alves <palves@redhat.com>
Cc: <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues
Date: Tue, 22 May 2012 00:05:00 -0000	[thread overview]
Message-ID: <alpine.DEB.1.10.1205220050470.11227@tp.orcam.me.uk> (raw)
In-Reply-To: <4FB6ACEF.2030600@redhat.com>

On Fri, 18 May 2012, Pedro Alves wrote:

> Ah, right, we can't return immediately --- but if we loop pread64/read,
> handling partial reads (and EINTR, while at it), until it returns error
> would get us the errno we want, I'd think.  I think the code would be
> more straightforward that way if it works, but anyway, just a suggestion;
> I'm super fine with your version.

 You're correct, especially EINTR is worth getting right, interrupted 
syscalls are always rare and unpredictable enough to make debugging their 
consequences a nightmare.

> >  Alternatively we could revamp the whole API to make 
> > the_target->read_memory return the number of bytes actually read, just 
> > like read and friends do.  That would of course ask for a complementing 
> > change to the_target->write_memory too.  I even thought about it when I 
> > finally tracked down what the cause of odd behaviour was, but I decided I 
> > was too tired debugging this issue already to turn gdbserver upside down 
> > at that stage.
> 
> 
> Yeah, certainly not worth the effort at this stage.

 I think ultimately we should do it though.  The /proc path is surely a 
later addition and can explain the resulting unfit API that was originally 
just fine with ptrace, but I think we should straighten it out and give an 
advantage to the common case.  Especially as it's not like the ptrace 
fallback or platforms where it's the only choice are going to suffer from 
such a read/write-like API.

 I think the two choices in the Linux handlers should be factored out into 
separate functions too.

> Thanks.  The whole patch is fine with me with this fix.

 Applied now, thanks for the review.

  Maciej


  reply	other threads:[~2012-05-22  0:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-16 22:57 Maciej W. Rozycki
2012-05-18 16:53 ` Pedro Alves
2012-05-18 18:46   ` Maciej W. Rozycki
2012-05-18 20:11     ` Pedro Alves
2012-05-22  0:05       ` Maciej W. Rozycki [this message]
2012-05-22  8:04         ` Regression for gdbserver [Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues] Jan Kratochvil
2012-05-22 12:43           ` Maciej W. Rozycki
2012-05-22 19:35             ` [patch] " Jan Kratochvil
2012-05-22 20:06               ` Maciej W. Rozycki
2012-05-22 20:42                 ` Jan Kratochvil
2012-05-22 23:34                   ` Maciej W. Rozycki
2012-05-23  5:29                     ` Jan Kratochvil

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.1.10.1205220050470.11227@tp.orcam.me.uk \
    --to=macro@codesourcery.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