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

Tristan Gingold
2012-02-15T08:05:59Z

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.