[Mesa-dev] [PATCH 3/3] u_dynarray: turn util_dynarray_{grow, resize} into element-oriented macros

[Haehnle, Nicolai]

On 05.05.19 01:19, Bas Nieuwenhuizen wrote:
>>   /* use util_dynarray_trim to reduce the allocated storage */
>>   static inline void *
>> -util_dynarray_resize(struct util_dynarray *buf, unsigned newsize)
>> +util_dynarray_resize_bytes(struct util_dynarray *buf, unsigned nelts, size_t eltsize)
>>   {
>> +   if (unlikely(nelts > UINT_MAX / eltsize)) {
>> +      util_dynarray_fini(buf);
>> +      return 0;
> 
> 
> Can we not do the util_dynarray_fini? I really like the principle that
> if we fail then nothing is modified and this deviates form that.

I guess there are two different schools of thought on this. Your way 
makes sense if callers actually test the return value -- but if they 
don't, this way is worse because the caller is subsequently very likely 
to overwrite random memory instead of just crashing.


> Also if someone handles the error seriously there is a large probably
> that the containing structure is going to be destructed which probably
> expexts a valid dynarray. If nobody handles the error (see e.g. the
> below util_dynarray_clone), then things are going to blow up either
> way.

util_dynarray_fini leaves the array in a valid state, so this is not a 
concern.

Cheers,
Nicolai


> 
>> +   }
>> +
>> +   unsigned newsize = nelts * eltsize;
>>      void *p = util_dynarray_ensure_cap(buf, newsize);
>>      buf->size = newsize;
>>
>>      return p;
>>   }
>>
>>   static inline void
>>   util_dynarray_clone(struct util_dynarray *buf, void *mem_ctx,
>>                       struct util_dynarray *from_buf)
>>   {
>>      util_dynarray_init(buf, mem_ctx);
>> -   util_dynarray_resize(buf, from_buf->size);
>> +   util_dynarray_resize_bytes(buf, 1, from_buf->size);
>>      memcpy(buf->data, from_buf->data, from_buf->size);
>>   }
>>
>>   static inline void *
>> -util_dynarray_grow(struct util_dynarray *buf, int diff)
>> +util_dynarray_grow_bytes(struct util_dynarray *buf, unsigned ngrow, size_t eltsize)
>>   {
>> -   return util_dynarray_resize(buf, buf->size + diff);
>> +   unsigned growbytes = ngrow * eltsize;
>> +
>> +   if (unlikely(ngrow > (UINT_MAX / eltsize) ||
>> +                growbytes > UINT_MAX - buf->size)) {
>> +      util_dynarray_fini(buf);
> 
> Can we not do the util_dynarray_fini, see above?
> 
>> +      return 0;
>> +   }
>> +
>> +   unsigned newsize = buf->size + growbytes;
>> +   void *p = util_dynarray_ensure_cap(buf, newsize);
>> +   buf->size = newsize;
>> +
>> +   return p;
>>   }
>>
>>   static inline void
>>   util_dynarray_trim(struct util_dynarray *buf)
>>   {
>>      if (buf->size != buf->capacity) {
>>         if (buf->size) {
>>            if (buf->mem_ctx) {
>>               buf->data = reralloc_size(buf->mem_ctx, buf->data, buf->size);
>>            } else {
>> @@ -146,21 +159,24 @@ util_dynarray_trim(struct util_dynarray *buf)
>>               ralloc_free(buf->data);
>>            } else {
>>               free(buf->data);
>>            }
>>            buf->data = NULL;
>>            buf->capacity = 0;
>>         }
>>      }
>>   }
>>
>> -#define util_dynarray_append(buf, type, v) do {type __v = (v); memcpy(util_dynarray_grow((buf), sizeof(type)), &__v, sizeof(type));} while(0)
>> +#define util_dynarray_append(buf, type, v) do {type __v = (v); memcpy(util_dynarray_grow_bytes((buf), 1, sizeof(type)), &__v, sizeof(type));} while(0)
>> +/* Returns a pointer to the space of the first new element (in case of growth) or NULL on failure. */
>> +#define util_dynarray_resize(buf, type, nelts) util_dynarray_resize_bytes(buf, (nelts), sizeof(type))
>> +#define util_dynarray_grow(buf, type, ngrow) util_dynarray_grow_bytes(buf, (ngrow), sizeof(type))
>>   #define util_dynarray_top_ptr(buf, type) (type*)((char*)(buf)->data + (buf)->size - sizeof(type))
>>   #define util_dynarray_top(buf, type) *util_dynarray_top_ptr(buf, type)
>>   #define util_dynarray_pop_ptr(buf, type) (type*)((char*)(buf)->data + ((buf)->size -= sizeof(type)))
>>   #define util_dynarray_pop(buf, type) *util_dynarray_pop_ptr(buf, type)
>>   #define util_dynarray_contains(buf, type) ((buf)->size >= sizeof(type))
>>   #define util_dynarray_element(buf, type, idx) ((type*)(buf)->data + (idx))
>>   #define util_dynarray_begin(buf) ((buf)->data)
>>   #define util_dynarray_end(buf) ((void*)util_dynarray_element((buf), char, (buf)->size))
>>   #define util_dynarray_num_elements(buf, type) ((buf)->size / sizeof(type))
>>
>> --
>> 2.20.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev