Re: RFA: New port: ia64-hp-openvms (3/3) - ia64-vms-tdep.c

[Tristan Gingold <gingold@adacore.com>]

On Feb 14, 2012, at 6:50 PM, Pedro Alves wrote:

First, thank you for your review.  I am currently addressing your points.

[…]

> 
>> +  info_len = extract_unsigned_integer (buf + 0, 4, byte_order);
>> +  pi->unwind_info_size = 8 * info_len;
>> +
>> +  /* Read info.  */
>> +  pi->unwind_info = malloc (pi->unwind_info_size);
> 
> xmalloc

Humm, do we want to exit abruptly in case of memory error ?  Note that we test the return status.
(OTOH, if malloc fails here, it is likely that the next xmalloc will too).

> 
>> +  if (!pi->unwind_info)
>> +    return -UNW_ENOMEM;
>> +
>> +  res = target_read_memory (table_addr + 8,
>> +                            pi->unwind_info, pi->unwind_info_size);
>> +  if (res != 0)
>> +    {
>> +      free (pi->unwind_info);
> 
> xfree
> 
>> +      pi->unwind_info = NULL;
>> +      return -UNW_ENOINFO;
>> +    }
>> +
>> +  /* FIXME: OSSD.  */
> 
> What's this?  What's OSSD?  What's left to fix?  Can we fix it?

OS Specific Data.  That's an extension of unwinding info on VMS, that is not handled by libunwind.
OTOH, I haven't seen them currently.  I suppose they are used only in very specific context (kernel ?)

>> +
>> +  pi->lsda = table_addr + 8 + pi->unwind_info_size;
>> +  if (extract_unsigned_integer (buf + 4, 2, byte_order) & 3)
>> +    {
>> +      pi->lsda += 8;
>> +      pi->handler = 0; /* FIXME: wrong, but who cares.  */
> 
> I don't know.  But I think this needs a better comment.

Sure.

[…]

> 
> Otherwise looks fine.  I can't really comment on the vms or ia64 specifics.
> 
> Just please make sure an --enable-targets=all build doesn't break
> unexpectedly.

Will do.

Thanks,
Tristan.