Re: [PATCH 3/3 v3] Implement remote_bfd_iovec_stat using vFile:fstat

[Gary Benson <gbenson@redhat.com>]

Pedro Alves wrote:
> On 03/10/2015 03:51 PM, Gary Benson wrote:
> > +/* Unpack an fio_uint_t.  */
> > +
> > +static unsigned int
> > +remote_fileio_to_host_uint (fio_uint_t fnum)
> > +{
> > +  gdb_byte *buf, *lim;
> > +  unsigned int num = 0;
> > +
> > +  for (buf = (gdb_byte *) fnum, lim = buf + 4; buf < lim; buf++)
> > +    {
> > +      num <<= 8;
> > +      num |= *buf;
> > +    }
> 
> How about we use
> 
>   extract_unsigned_integer ((gdb_byte *) fnum, 4, BFD_ENDIAN_BIG)
> 
> instead?

Ok.

> > +   into ST.  Return 0 on success, or -1 if an error occurs (and
> > +   set *REMOTE_ERRNO).  */
> > +
> > +static int
> > +remote_hostio_fstat (struct target_ops *self,
> > +		     int fd, struct stat *st,
> > +		     int *remote_errno)
> > +{
> > +  struct remote_state *rs = get_remote_state ();
> > +  char *p = rs->buf;
> > +  int left = get_remote_packet_size ();
> > +  int attachment_len, ret;
> > +  char *attachment;
> > +  struct fio_stat fst;
> > +  int read_len;
> > +
> > +  if (packet_support (PACKET_vFile_fstat) != PACKET_ENABLE)
> > +    {
> > +      memset (st, 0, sizeof (struct stat));
> > +      st->st_size = INT_MAX;
> 
> A future reader may wonder why this isn't ENOSYS instead.  I think
> a comment here would help.

How about this:

      /* Strictly we should return -1, ENOSYS here, but when
	 "set sysroot remote:" was implemented in August 2008
	 BFD's need for a stat function was sidestepped with
	 this hack.  This was not remedied until March 2015
	 so we retain the previous behavior to avoid breaking
	 compatibility.

	 Note that the memset is a March 2015 addition; older
	 GDBs set st_size *and nothing else* so the structure
	 would have garbage in all other fields.  This might
	 break something but retaining the previous behavior
	 here would be just too wrong.  */

Are you ok for me to commit this patch, reordered before the
gdbserver changes, with extract_unsigned_integer used in
remote_fileio_to_host_{uint,ulong}, and that comment added?
(I will regenerate patch 2 with updated docs for Eli to
approve).

Thanks,
Gary