On Mon, Sep 17, 2012 at 2:49 AM, Florian Weimer <fwei...@redhat.com> wrote:
> On 09/05/2012 07:31 AM, Ian Lance Taylor wrote:
>>
>> On Wed, Aug 29, 2012 at 10:32 AM, Florian Weimer <f...@deneb.enyo.de> wrote:
>>>
>>>
>>> This patches fixes an integer overflow in libiberty, which leads to
>>> crashes in binutils.  The long version of the objalloc_alloc macro
>>> would have needed another conditional, so I removed that and replaced
>>> it with a call to the actual implementation.
>>
>>
>> I guess I don't see why removing the macro is desirable.  In many uses
>> of objalloc_alloc the conditional can be optimized out anyhow.  It's
>> true that it can't always be, but, so what?  The macro is probably
>> still a win.
>
>
> Fair enough.  I've added a wraparound check to the macro.  Okay for trunk?

>  {
> +  unsigned long len = original_len;
>    /* We avoid confusion from zero sized objects by always allocating
>       at least 1 byte.  */

Please add a blank line after the variable declaration.

> -     (__len <= __o->current_space                                    \
> +     (__len && __len <= __o->current_space                           \

Please write __len != 0 or len > 0.

This is OK with those changes.

Thanks.

Ian

Reply via email to