Re: [PATCH V2] vector: return false if realloc fails in vector_alloc_slot func

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2020/8/11 17:46, Martin Wilck wrote:
> On Tue, 2020-08-11 at 11:23 +0800, Zhiqiang Liu wrote:
> 
>> diff --git a/libmultipath/vector.c b/libmultipath/vector.c
>> index 501cf4c5..39e2c20f 100644
>> --- a/libmultipath/vector.c
>> +++ b/libmultipath/vector.c
>> @@ -35,26 +35,22 @@ vector_alloc(void)
>>  }
>>
>>  /* allocated one slot */
>> -void *
>> +bool
>>  vector_alloc_slot(vector v)
>>  {
>>  	void *new_slot = NULL;
>>
>>  	if (!v)
>> -		return NULL;
>> -
>> -	v->allocated += VECTOR_DEFAULT_SIZE;
>> -	if (v->slot)
>> -		new_slot = REALLOC(v->slot, sizeof (void *) * v-
>>> allocated);
>> -	else
>> -		new_slot = (void *) MALLOC(sizeof (void *) * v-
>>> allocated);
>> +		return false;
>>
>> +	new_slot = REALLOC(v->slot, sizeof (void *) * (v->allocated +
>> VECTOR_DEFAULT_SIZE));
> 
> Please wrap this line. We basically still use a 80-columns limit, with
> the exception of string constants (mostly condlog() lines). We don't
> strictly enforce it, but 92 columns is a bit too much for my taste.
Thanks for your reply.
I will introduce new var to shorten this line and init new slot with
using loop in the v3 patch.

Thanks again.

Regards.
Zhiqiang Liu

> 
>>  	if (!new_slot)
>> -		v->allocated -= VECTOR_DEFAULT_SIZE;
>> -	else
>> -		v->slot = new_slot;
>> +		return false;
>>
>> -	return v->slot;
>> +	v->slot = new_slot;
>> +	v->allocated += VECTOR_DEFAULT_SIZE;
>> +	v->slot[VECTOR_SIZE(v) - 1] = NULL;
> 
> This assumes that VECTOR_DEFAULT_SIZE == 1. While that has been true
> essentially forever, there's no guarantee for the future. Better use a
> for loop, or memset().
> 
> Regards
> Martin
> 
> 
> 
> .
> 

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux