Small remote file transfer protocol adition

Small remote file transfer protocol adition

[Pedro Alves]

Hi all,

For the remote file transfer support added recently to gdb
head, I would like to be able to optionally pass the
native error back to gdb in addition to the current
error values supported.

The reply packet is currently:

F$(retcode),$(remote_error);

where remote_error is one of the FILEIO_XXX defines defined
in src/include/gdb.h :

/* errno values */
#define FILEIO_EPERM                1
#define FILEIO_ENOENT               2
#define FILEIO_EINTR                4
#define FILEIO_EIO                  5
#define FILEIO_EBADF                9
(...)
#define FILEIO_ENOSYS              88
#define FILEIO_ENAMETOOLONG        91
#define FILEIO_EUNKNOWN          9999

These values clearly map to a subset of the possible values of
errno.  However, it doesn't map all the possible values.  On the
other hand, there is at least one supported target where
the mapping is awkward, and lossy.  Perhaps not suprisingly,
it is a Windows based target -- Windows CE.  This target has
no concept of errno in the C runtime; one must use the standard
Win32 API GetLastError/SetLastError calls to read/write error codes,
which aren't few.  It is easy to imagine other targets out there
that would want to pass more finer grained error codes back to
the user which aren't covered by the FILEIO_* constants.

To be able support this, this proposal adds an optional field
to the reply packet:

F$(retcode),$(remote_error),$(native_error);

By keeping the remote_error in place, gdb common code
can continue to be agnostic to which target it is talking
to.  In the future, if more advanced and automatic (from GDB's
perspective) file error handling is needed, GDB will always
use the remote_error field.  The native_error field's
only purpose is to report back to the user an error code that
is as accurate as possible, instead of displaying
an uninformative "unknown error".

Like the remote_error field, the native_error is formatted
as an hexadecimal number.

Please note that this field, if added, will be optional, and that
there is currently no GDB release that has remote file transfer
support.  That support was added after 6.7 was released.

Would that be OK?

-- 
Pedro Alves

Re: Small remote file transfer protocol adition

[Daniel Jacobowitz]

On Tue, Jan 29, 2008 at 04:28:31AM +0000, Pedro Alves wrote:
> To be able support this, this proposal adds an optional field
> to the reply packet:
>
> F$(retcode),$(remote_error),$(native_error);

I don't like designing an interface that is going to have to show hex
numbers to the user.  They'd have to figure out that it was a target
error number and look it up themselves.  Despite the localization
issues involved, do you think it might be better to add an error
string if you need the extra detail?  We could put it in the
attachment field.

Could you give me an example of a useful error code which does not map
to one of the POSIX errors?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Small remote file transfer protocol adition

[Pedro Alves]

 >On Tue, Jan 29, 2008 at 04:28:31AM +0000, Pedro Alves wrote:
 >> To be able support this, this proposal adds an optional field
 >> to the reply packet:
 >>
 >> F$(retcode),$(remote_error),$(native_error);
 >
 >I don't like designing an interface that is going to have to show hex
 >numbers to the user.  They'd have to figure out that it was a target
 >error number and look it up themselves.

We could state it clearly in the error message:

With native error:
  Unkown remote I/O error.  Target error was: 0xc0454054
  Remote I/O error: $(strerror(host_error)).  Target error was: 161

Without:
  Unkown remote I/O error.
  Remote I/O error: $(strerror(host_error)).

 > Despite the localization
 >issues involved, do you think it might be better to add an error
 >string if you need the extra detail?  We could put it in the
 >attachment field.

I missed that field.  I guess it would work too.

 >Could you give me an example of a useful error code which does not map
 >to one of the POSIX errors?

Most regular ones map correctly, but you lose detail.
Win32 codes are more finer grained then posix error codes.

E.g. for my preliminary patch:

     case ERROR_ACCESS_DENIED:
     case ERROR_SHARING_VIOLATION:
       return FILEIO_EACCES;

     case ERROR_FILENAME_EXCED_RANGE:
     case ERROR_INVALID_DATA:
     case ERROR_INVALID_PARAMETER:
     case ERROR_NEGATIVE_SEEK:
       return FILEIO_EINVAL;

Then, there's always the unexpected error that happens once,
and is hard to reproduce.  winerr.h from w32api defines around
1500 error codes.  Granted, most aren't file io related, but
I'm sure I'll miss one or two in the mapping.  I'd rater see it
in the error message when it happens.

Currently, in this situation we get:
  Unknown remote I/O error.

Not helpful, scarry, and these missing mappings tend to
not get fixed.

Would a patch that implements support for the attachment
field in an error reply be accepted, in principle?
I'll give it a try.

-- 
Pedro Alves

Re: Small remote file transfer protocol adition

[Daniel Jacobowitz]

On Tue, Jan 29, 2008 at 03:20:54PM +0000, Pedro Alves wrote:
> Would a patch that implements support for the attachment
> field in an error reply be accepted, in principle?
> I'll give it a try.

Well, do you think it's a good idea?  I can't see anything wrong with
it, but then, I just made it up.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Small remote file transfer protocol adition

[Pedro Alves]

Daniel Jacobowitz wrote:
> On Tue, Jan 29, 2008 at 03:20:54PM +0000, Pedro Alves wrote:
>> Would a patch that implements support for the attachment
>> field in an error reply be accepted, in principle?
>> I'll give it a try.
> 

> Well, do you think it's a good idea?  I can't see anything wrong with
> it, but then, I just made it up.
> 

Well, since you ask :-)  I'd prefer my first suggestion, just
because it's simpler.  No worries about buffer size
limits, and the message being truncated.  If passing a message,
I'd have to be careful with what I'd say there, so it
doesn't confuse the user (strerror says one thing, native
error says something similar but not the same, possibly in
different locales).  This probably means I'd just pass the error
number anyway.

-- 
Pedro Alves

Re: Small remote file transfer protocol adition

[Daniel Jacobowitz]

On Tue, Jan 29, 2008 at 05:51:58PM +0000, Pedro Alves wrote:
> Well, since you ask :-)  I'd prefer my first suggestion, just
> because it's simpler.  No worries about buffer size
> limits, and the message being truncated.  If passing a message,
> I'd have to be careful with what I'd say there, so it
> doesn't confuse the user (strerror says one thing, native
> error says something similar but not the same, possibly in
> different locales).  This probably means I'd just pass the error
> number anyway.

Yes, I see.  I'm convinced.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Small remote file transfer protocol adition

[Pedro Alves]

Daniel Jacobowitz wrote:
> On Tue, Jan 29, 2008 at 05:51:58PM +0000, Pedro Alves wrote:
>> Well, since you ask :-)  I'd prefer my first suggestion, just
>> because it's simpler.  No worries about buffer size
>> limits, and the message being truncated.  If passing a message,
>> I'd have to be careful with what I'd say there, so it
>> doesn't confuse the user (strerror says one thing, native
>> error says something similar but not the same, possibly in
>> different locales).  This probably means I'd just pass the error
>> number anyway.
> 
> Yes, I see.  I'm convinced.
> 

Proposed patch posted here:
http://sourceware.org/ml/gdb-patches/2008-01/msg00860.html

-- 
Pedro Alves