Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Pedro Alves]

Hi!

On 09/05/2013 08:29 PM, Pedro Alves wrote:
>> > +static int
>> > +gnu_read_memory (CORE_ADDR addr, unsigned char *myaddr, int length)
>> > +{
>> > +  int ret = 0;
>> > +  task_t task = (gnu_current_inf
>> > + ? (gnu_current_inf->task
>> > +    ? gnu_current_inf->task->port : 0) : 0);
>> > +  if (task == MACH_PORT_NULL)
>> > +    return 0;
>> > +  ret = gnu_read_inferior (task, addr, myaddr, length);
>> > +  if (length != ret)
>> > +    {
>> > +      gnu_debug ("gnu_read_inferior,length=%d, but return %d\n", length, 
>> > ret);
>> > +      return -1;
>> > +    }
>> > +  return 0;
>> > +}
>> > +
>> > +static int
>> > +gnu_write_memory (CORE_ADDR addr, const unsigned char *myaddr, int length)
>> > +{
>> > +  int ret = 0;
>> > +  task_t task = (gnu_current_inf
>> > + ? (gnu_current_inf->task
>> > +    ? gnu_current_inf->task->port : 0) : 0);
>> > +  if (task == MACH_PORT_NULL)
>> > +    return 0;
>> > +  ret = gnu_write_inferior (task, addr, myaddr, length);
>> > +  if (length != ret)
>> > +    {
>> > +      gnu_debug ("gnu_write_inferior,length=%d, but return %d\n", length,
>> > + ret);
>> > +      return -1;
>> > +    }
>> > +  return 0;
>> > +}
>> > +
> These should reall be wrappers around a common shared function.  I have
> a patch that I think helps here.  I'll post it in soon.
> 

This is what I meant:
https://sourceware.org/ml/gdb-patches/2013-09/msg00253.html

I was thinking you'd wrap gnu_xfer_memory.

But I have to say I don't really understand the real need for
all those:

  task_t task = (gnu_current_inf
                 ? (gnu_current_inf->task
                    ? gnu_current_inf->task->port : 0)
                 : 0);
  int res;

  if (task == MACH_PORT_NULL)
    return 0;

checks in the existing code.  I mean, why would we reach here with
an invalid inferior/task/port selected?
It just reads as workaround for some bug to me.

Thanks,
-- 
Pedro Alves