This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues


On 05/18/2012 07:45 PM, Maciej W. Rozycki wrote:

>> > So how about just returning immediately if reading from /proc manages to
>> > read something?  From what you say, the PEEKTEXT fallback then won't just
>> > normally fail; it'll _always_ fail.  I suspect that'd make the code a bit
>> > simpler.
>  Maybe.  Question is, actually why it was written like this in the first 
> place.  And the only answer I could come up with was: to retrieve the 
> right error code for the caller to investigate.  You won't get that with 
> pread64/read if they succeed reading any data at all, even if the amount 
> is less than requested.  Hence I decided to preserve the original flow.  
> I do not feel comfortable with cooking up an artificial value of errno.


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.

> 
>  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.

>> > You shouldn't assume that "errno" is preserved across library
>> > calls (memcpy in this case).
>  Oh, that was an oversight rather than a deliberate assumption, sorry 
> about that.  Here's a trivial update that I have applied to my fix.


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

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]