Re: [Accel-config] [PATCH] accel-config: Fixes for pedantic compiler warnings

On 8/24/2023 1:35 AM, King, Colin wrote:
> The xrealloc warning is a genuine issue where realloc fails, take the example:
> 
> 	ret = realloc(ptr, size);
> 
> if ret is NULL then the reallocation failed it is important to realize ptr is still valid and you end up with a leak on ptr.
> 
> The function also handles the zero size realloc with a realloc attempt and then another attempt with a size 1.

The size = 1 is a meaningless hack inherited from the original version 
of this function taken from git tool source, which has changed in the 
latest version. The new version frees the ptr if size == 0.

Based on usage, this code should simply call die() on failure. See below 
for more explanation on usage.

Following code can be modified as below
> 
> I suspect the function should be more like:
> 
> void *xrealloc(void *ptr, size_t size)
> {
> 	void *ret;
> 
> 	if (!size)
> 		size = 1;

Above check is not necessary

> 	ret = realloc(ptr, size);
> 	if (!ret) {
> 		free(ptr);

I think free is not necessary, but no harm in leaving it. Static 
analysis tools find the NORETURN in the die() path and don't complain.

> 		die("Out of memory, realloc failed");	
> 	}
> 	return ret;
> }
> 
> The free() is academic since die() terminates the code, but at least it removes any doubt from static analysis tools that ptr is not being leaked

The callers don't handle a NULL pointer return or a freed pointer 
returned by malloc(0) (if size==0 was passed to free the buffer), so 
nothing other than die() matters. Also don't see anywhere it is getting 
called with size==0.

Thanks,
Ramesh