On Mon, Sep 17, 2012 at 2:49 AM, Florian Weimer <[email protected]> wrote:
> On 09/05/2012 07:31 AM, Ian Lance Taylor wrote:
>>
>> On Wed, Aug 29, 2012 at 10:32 AM, Florian Weimer <[email protected]> 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