Hi Pete, thanks for the review.
On 09/05/2014 06:48 PM, Pete Zaitcev wrote:
> On Fri, 05 Sep 2014 10:39:28 +0200
> Alexander Kurpiers <[email protected]> wrote:
>
>> @@ -1707,9 +1709,14 @@ static int _rtlsdr_alloc_async_buffers(rtlsdr_dev_t
>> *dev)
>> if (!dev->xfer_buf) {
>> dev->xfer_buf = malloc(dev->xfer_buf_num *
>> sizeof(unsigned char *));
>> + if (dev->xfer_buf == NULL)
>> + return -1;
>>
>> - for(i = 0; i < dev->xfer_buf_num; ++i)
>> + for(i = 0; i < dev->xfer_buf_num; ++i){
>> dev->xfer_buf[i] = malloc(dev->xfer_buf_len);
>> + if (dev->xfer_buf[i] == NULL)
>> + return -1;
>> + }
>> }
> Please don't do stuff like that. And I'm not just talking about
> the wrong indentation. If a function runs a bunch of allocs inside,
> and one of them fails, it must unroll those that succeeded before
> returning the error to the caller. Failure to do it is why you
> had to add stuff like this:
>
True - I only ported the original code from Leif without paying
attention (I only worked myself on 0002-0005).
Sorry, I should have noticed.
>> - _rtlsdr_alloc_async_buffers(dev);
>> + /* Check the error code. Free return if errors */
>> + if(_rtlsdr_alloc_async_buffers(dev) < 0) {
>> + _rtlsdr_free_async_buffers(dev);
>> + return -1;
>> + }
> Just say no to programming like this. The object is either initialized
> or not initialized, and never half-way initialized.
>
I agree. I'd say we drop the whole patch 0006 and do it properly.
I'll have a look later.
