On Wed, Nov 27, 2013 at 8:04 PM, Dan Carpenter <dan.carpen...@oracle.com> wrote:
> On Wed, Nov 27, 2013 at 07:37:30PM +0800, ZHAO Gang wrote:
>> On Wed, Nov 27, 2013 at 5:02 PM, Dan Carpenter <dan.carpen...@oracle.com> 
>> wrote:
>> > On Wed, Nov 27, 2013 at 03:45:12PM +0800, ZHAO Gang wrote:
>> >> @@ -2208,8 +2204,11 @@ static int et131x_rx_dma_memory_alloc(struct 
>> >> et131x_adapter *adapter)
>> >>       rx_ring = &adapter->rx_ring;
>> >>
>> >>       /* Alloc memory for the lookup table */
>> >> -     rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL);
>> >> -     rx_ring->fbr[1] = kmalloc(sizeof(struct fbr_lookup), GFP_KERNEL);
>> >> +     rx_ring->fbr[0] = kmalloc(sizeof(struct fbr_lookup) * 2, 
>> >> GFP_KERNEL);
>> >> +     if (!rx_ring->fbr[0])
>> >> +             return -ENOMEM;
>> >> +
>> >> +     rx_ring->fbr[1] = rx_ring->fbr[0] + 1;
>> >>
>> >>       /* The first thing we will do is configure the sizes of the buffer
>> >>        * rings. These will change based on jumbo packet support.  Larger
>> >
>> > I can't make myself review any further beyond this point...  Please
>> > don't do this nonsense.
>> >
>>
>> I don't think it is nonsense. The original code doesn't have error
>> check on this two kmalloc, I combine them to one so I only need to add
>> one error check on it.
>
> Just do two allocations and two checks unless you have benchmark numbers
> to show that it faster.  Making the code so complicated for no reason is
> a wrong thing.
>

I don't do this for benchmark, I do this for I just want to write less
code. Say complication, you said it's nonsense, I think you did mean
idiotic nonsense, not complicated nonsense :-)

Anyway, if it must be changed, I will change it.

> regards,
> dan carpenter
>
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to