Re: [PATCH] Document short responses to qXfer:object:read.

Re: [PATCH] Document short responses to qXfer:object:read.

[David Taylor]

> From: Eli Zaretskii <eliz at gnu dot org>
> Date: Thu, 26 May 2016 22:31:40 +0300
> 
> > From: David Taylor <dtaylor@emc.com>
> > cc: dtaylor@emc.com
> > Date: Thu, 26 May 2016 14:10:37 -0400
> > 
> > +2016-05-26  David Taylor  <dtaylor@emc.com>
> > +
> > +	* gdb.texinfo (General Query Packets): Document treatment of short
> > +	qXfer responses and EOF indication.
> > +
> >  2016-05-23  Tom Tromey  <tom@tromey.com>
> >  
> >  	* python.texi (Basic Python): Document gdb.breakpoints return.
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index 8f1ad4e..c674c7a 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -37251,6 +37251,11 @@ starting at @var{offset} bytes into the data.  The content and
> >  encoding of @var{annex} is specific to @var{object}; it can supply
> >  additional details about what data to access.
> >  
> > +The target is permitted to return a short response.  The response is
> > +considered to cover the range @var{offset} (inclusive) to @var{offset}
> > ++ @var{response length} (exclusive).  End of file is indicated by a
> > +zero length response.
> > +
> >  Here are the specific requests of this form defined so far.  All
> >  @samp{qXfer:@var{object}:read:@dots{}} requests use the same reply
> >  formats, listed below.
> 
> Thanks.  However, I have difficulty understanding what this text is
> trying to convey.  Problematic words are "permitted" and "short" --
> they don't seem to be related to anything around this text, and are
> not explained by the text itself.  I also don't see why EOF is
> important here, or even related.
> 
> Could you perhaps explain in your own words what is this about, and
> what the text lacks without this paragraph?  I might then propose a
> more clear wording.
> 
> Thanks.

The qXfer:object:read packet contains an offset and a length.  It says
for <object>, starting at offset <offset>, send me <length> characters.

When 'reading' object <object>, GDB starts at offset 0 and sends
qXfer:<object>:read packets with progressively higher offsets until it
gets everything.

If it was a file, then a read of less than the requested amount (aka a
short read) would be interpreted as having reached the end of the file.

These objects aren't really files.  And returning less than requested
does not mean that the end of the object has been reached.

GDB's next 'read' from 'object' will be at offset

    <offset> + <returned amount>

NOT at

    <offset> + <requested amount>

The fact that sending back less than requested is okay (i.e., does not
say to GDB -- hey, that's it, there is no more) and that the next
request starts where the last response ended (i.e., offset + returned
amount) rather than where the last request ended (i.e., offset +
requested amount) makes life easier on the remote stub implementer.

Hope that clarifies somewhat.  Now, can we come up with better wording?

David

Re: [PATCH] Document short responses to qXfer:object:read.

[Eli Zaretskii]

> From: David Taylor <dtaylor@emc.com>
> cc: gdb-patches@sourceware.org, dtaylor@emc.com
> Date: Thu, 26 May 2016 16:33:27 -0400
> 
> The qXfer:object:read packet contains an offset and a length.  It says
> for <object>, starting at offset <offset>, send me <length> characters.
> 
> When 'reading' object <object>, GDB starts at offset 0 and sends
> qXfer:<object>:read packets with progressively higher offsets until it
> gets everything.
> 
> If it was a file, then a read of less than the requested amount (aka a
> short read) would be interpreted as having reached the end of the file.
> 
> These objects aren't really files.  And returning less than requested
> does not mean that the end of the object has been reached.
> 
> GDB's next 'read' from 'object' will be at offset
> 
>     <offset> + <returned amount>
> 
> NOT at
> 
>     <offset> + <requested amount>
> 
> The fact that sending back less than requested is okay (i.e., does not
> say to GDB -- hey, that's it, there is no more) and that the next
> request starts where the last response ended (i.e., offset + returned
> amount) rather than where the last request ended (i.e., offset +
> requested amount) makes life easier on the remote stub implementer.
> 
> Hope that clarifies somewhat.

It very much does, thanks.  I have 3 follow-up questions:

 . It's not clear to me what is the "returned amount" here, as the
   listed responses for this packet don't seem to provide the amount
   of data in the response.  Do you mean the length of the data in the
   "m DATA" or "l DATA" responses, counted in bytes?

 . What is a zero-length response?  Is that "l" without data?

 . If my interpretation of the above 2 issues is correct, then why do we
   need this new text you suggest?  The 'm' and 'l' responses already
   say that the data can be shorter than the request, which seems to
   be what the new text says.

Finally, it looks to me that we should move the description of the
responses to precede the details of the 'read' requests, since the
list of those requests is very long, and thus the promised description
of the responses "below" is very far away, which IMO gets in the way
of reading and comprehending the issue.

> Now, can we come up with better wording?

After we've figured out the above left-overs, sure.

Thanks.

Re: [PATCH] Document short responses to qXfer:object:read.

[Pedro Alves]

On 05/27/2016 09:35 AM, Eli Zaretskii wrote:

> Finally, it looks to me that we should move the description of the
> responses to precede the details of the 'read' requests, since the
> list of those requests is very long, and thus the promised description
> of the responses "below" is very far away, which IMO gets in the way
> of reading and comprehending the issue.

Fully agreed.

Thanks,
Pedro Alves

Re: [PATCH] Document short responses to qXfer:object:read.

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 27 May 2016 09:55:03 +0100
> 
> On 05/27/2016 09:35 AM, Eli Zaretskii wrote:
> 
> > Finally, it looks to me that we should move the description of the
> > responses to precede the details of the 'read' requests, since the
> > list of those requests is very long, and thus the promised description
> > of the responses "below" is very far away, which IMO gets in the way
> > of reading and comprehending the issue.
> 
> Fully agreed.

The patch for that is below; OK?

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 34b9527..3abec90 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2016-05-27  Eli Zaretskii  <eliz@gnu.org>
+
+	* gdb.texinfo (General Query Packets): Move the description of the
+	response before the long list of the specific 'read' requests.
+
 2016-05-23  Tom Tromey  <tom@tromey.com>
 
 	* python.texi (Basic Python): Document gdb.breakpoints return.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8f1ad4e..9310365 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37251,9 +37251,40 @@
 encoding of @var{annex} is specific to @var{object}; it can supply
 additional details about what data to access.
 
+Reply:
+@table @samp
+@item m @var{data}
+Data @var{data} (@pxref{Binary Data}) has been read from the
+target.  There may be more data at a higher address (although
+it is permitted to return @samp{m} even for the last valid
+block of data, as long as at least one byte of data was read).
+It is possible for @var{data} to have fewer bytes than the @var{length} in the
+request.
+
+@item l @var{data}
+Data @var{data} (@pxref{Binary Data}) has been read from the target.
+There is no more data to be read.  It is possible for @var{data} to
+have fewer bytes than the @var{length} in the request.
+
+@item l
+The @var{offset} in the request is at the end of the data.
+There is no more data to be read.
+
+@item E00
+The request was malformed, or @var{annex} was invalid.
+
+@item E @var{nn}
+The offset was invalid, or there was an error encountered reading the data.
+The @var{nn} part is a hex-encoded @code{errno} value.
+
+@item @w{}
+An empty reply indicates the @var{object} string was not recognized by
+the stub, or that the object does not support reading.
+@end table
+
 Here are the specific requests of this form defined so far.  All
 @samp{qXfer:@var{object}:read:@dots{}} requests use the same reply
-formats, listed below.
+formats, listed above.
 
 @table @samp
 @item qXfer:auxv:read::@var{offset},@var{length}
@@ -37458,37 +37489,6 @@
 
 @end table
 
-Reply:
-@table @samp
-@item m @var{data}
-Data @var{data} (@pxref{Binary Data}) has been read from the
-target.  There may be more data at a higher address (although
-it is permitted to return @samp{m} even for the last valid
-block of data, as long as at least one byte of data was read).
-It is possible for @var{data} to have fewer bytes than the @var{length} in the
-request.
-
-@item l @var{data}
-Data @var{data} (@pxref{Binary Data}) has been read from the target.
-There is no more data to be read.  It is possible for @var{data} to
-have fewer bytes than the @var{length} in the request.
-
-@item l
-The @var{offset} in the request is at the end of the data.
-There is no more data to be read.
-
-@item E00
-The request was malformed, or @var{annex} was invalid.
-
-@item E @var{nn}
-The offset was invalid, or there was an error encountered reading the data.
-The @var{nn} part is a hex-encoded @code{errno} value.
-
-@item @w{}
-An empty reply indicates the @var{object} string was not recognized by
-the stub, or that the object does not support reading.
-@end table
-
 @item qXfer:@var{object}:write:@var{annex}:@var{offset}:@var{data}@dots{}
 @cindex write data into object, remote request
 @anchor{qXfer write}

Re: [PATCH] Document short responses to qXfer:object:read.

[Pedro Alves]

On 05/27/2016 02:21 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 27 May 2016 09:55:03 +0100
>>
>> On 05/27/2016 09:35 AM, Eli Zaretskii wrote:
>>
>>> Finally, it looks to me that we should move the description of the
>>> responses to precede the details of the 'read' requests, since the
>>> list of those requests is very long, and thus the promised description
>>> of the responses "below" is very far away, which IMO gets in the way
>>> of reading and comprehending the issue.
>>
>> Fully agreed.
> 
> The patch for that is below; OK?

LGTM. 

I think that for symmetry, it be good to do the same to
qxfer:object:write, even though the list of specific
write requests isn't as large.

Thanks,
Pedro Alves

Re: [PATCH] Document short responses to qXfer:object:read.

[Eli Zaretskii]

> Cc: dtaylor@emc.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 27 May 2016 14:48:11 +0100
> 
> I think that for symmetry, it be good to do the same to
> qxfer:object:write, even though the list of specific
> write requests isn't as large.

Done and pushed, thanks.

Re: [PATCH] Document short responses to qXfer:object:read.

[Eli Zaretskii]

> From: David Taylor <dtaylor@emc.com>
> cc: gdb-patches@sourceware.org
> Date: Fri, 27 May 2016 12:51:59 -0400
> 
> > Finally, it looks to me that we should move the description of the
> > responses to precede the details of the 'read' requests, since the
> > list of those requests is very long, and thus the promised description
> > of the responses "below" is very far away, which IMO gets in the way
> > of reading and comprehending the issue.
> > 
> > > Now, can we come up with better wording?
> > 
> > After we've figured out the above left-overs, sure.
> > 
> > Thanks.
> 
> I didn't pay attention to that as it was far enough after the text in
> question and looked -- at quick glance -- like it was part of the osdata
> description and therefore not relevant to threads.
> 
> Taking a longer look, it clearly is relevant.
> 
> It addresses most of my concerns and moving it, as you suggested, would
> definitely, IMO, help.

I already moved it.  If you think something still needs to be
augmented there, please tell what that is.

Thanks.

Re: [PATCH] Document short responses to qXfer:object:read.

[David Taylor]

> From: Eli Zaretskii <eliz at gnu dot org>
> Date: Fri, 27 May 2016 11:35:27 +0300


> It very much does, thanks.  I have 3 follow-up questions:
> 
>  . It's not clear to me what is the "returned amount" here, as the
>    listed responses for this packet don't seem to provide the amount
>    of data in the response.  Do you mean the length of the data in the
>    "m DATA" or "l DATA" responses, counted in bytes?
> 
>  . What is a zero-length response?  Is that "l" without data?
> 
>  . If my interpretation of the above 2 issues is correct, then why do we
>    need this new text you suggest?  The 'm' and 'l' responses already
>    say that the data can be shorter than the request, which seems to
>    be what the new text says.
> 
> Finally, it looks to me that we should move the description of the
> responses to precede the details of the 'read' requests, since the
> list of those requests is very long, and thus the promised description
> of the responses "below" is very far away, which IMO gets in the way
> of reading and comprehending the issue.
> 
> > Now, can we come up with better wording?
> 
> After we've figured out the above left-overs, sure.
> 
> Thanks.

I didn't pay attention to that as it was far enough after the text in
question and looked -- at quick glance -- like it was part of the osdata
description and therefore not relevant to threads.

Taking a longer look, it clearly is relevant.

It addresses most of my concerns and moving it, as you suggested, would
definitely, IMO, help.

Thanks.

Re: [PATCH] Document short responses to qXfer:object:read.

[Eli Zaretskii]

> From: David Taylor <dtaylor@emc.com>
> cc: dtaylor@emc.com
> Date: Thu, 26 May 2016 14:10:37 -0400
> 
> +2016-05-26  David Taylor  <dtaylor@emc.com>
> +
> +	* gdb.texinfo (General Query Packets): Document treatment of short
> +	qXfer responses and EOF indication.
> +
>  2016-05-23  Tom Tromey  <tom@tromey.com>
>  
>  	* python.texi (Basic Python): Document gdb.breakpoints return.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 8f1ad4e..c674c7a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -37251,6 +37251,11 @@ starting at @var{offset} bytes into the data.  The content and
>  encoding of @var{annex} is specific to @var{object}; it can supply
>  additional details about what data to access.
>  
> +The target is permitted to return a short response.  The response is
> +considered to cover the range @var{offset} (inclusive) to @var{offset}
> ++ @var{response length} (exclusive).  End of file is indicated by a
> +zero length response.
> +
>  Here are the specific requests of this form defined so far.  All
>  @samp{qXfer:@var{object}:read:@dots{}} requests use the same reply
>  formats, listed below.

Thanks.  However, I have difficulty understanding what this text is
trying to convey.  Problematic words are "permitted" and "short" --
they don't seem to be related to anything around this text, and are
not explained by the text itself.  I also don't see why EOF is
important here, or even related.

Could you perhaps explain in your own words what is this about, and
what the text lacks without this paragraph?  I might then propose a
more clear wording.

Thanks.

[PATCH] Document short responses to qXfer:object:read.

[David Taylor]

[We have a copyright assignment on file (in case it matters -- but, this
is short enough that I doubt it is is copyrightable).]

This is a follow up to my May 9th posting (subject: qXfer:threads:read
packets) on the gdb mailing list.  This only addresses the short
response part, not the dtd vs documentation discrepancy.

 gdb/doc/ChangeLog   | 5 +++++
 gdb/doc/gdb.texinfo | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 34b9527..b46075f 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2016-05-26  David Taylor  <dtaylor@emc.com>
+
+	* gdb.texinfo (General Query Packets): Document treatment of short
+	qXfer responses and EOF indication.
+
 2016-05-23  Tom Tromey  <tom@tromey.com>
 
 	* python.texi (Basic Python): Document gdb.breakpoints return.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8f1ad4e..c674c7a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37251,6 +37251,11 @@ starting at @var{offset} bytes into the data.  The content and
 encoding of @var{annex} is specific to @var{object}; it can supply
 additional details about what data to access.
 
+The target is permitted to return a short response.  The response is
+considered to cover the range @var{offset} (inclusive) to @var{offset}
++ @var{response length} (exclusive).  End of file is indicated by a
+zero length response.
+
 Here are the specific requests of this form defined so far.  All
 @samp{qXfer:@var{object}:read:@dots{}} requests use the same reply
 formats, listed below.
-- 
1.9.1