Re: [PATCH i-g-t] tools/intel_gpu_top: prevent losing original pointer on realloc failure

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

Hi Tvrtko
On Tue Dec 16, 2025 at 8:02 PM CET, Tvrtko Ursulin wrote:
>
> On 15/12/2025 14:45, Sebastian Brzezinka wrote:
>> Previously, realloc was called directly on engines after incrementing
>> num_engines. If realloc failed, it would return NULL and overwrite the
>> original pointer, causing a memory leak and leaving engines inaccessible.
>> This patch uses a temporary pointer for realloc and only updates engines
>> after a successful allocation, ensuring the original pointer is preserved
>> on failure and preventing data loss.
>
> I don't see a memory leak, or any other problem really. If realloc fails 
> the flow is this:
>
> static struct engines *discover_engines(char *device)
> {
> ...
> 		engines = realloc(engines, sizeof(struct engines) +
> 				  engines->num_engines * sizeof(struct engine));
> 		if (!engines) {
> 			ret = errno;
> 			break;
> 		}
> ...
> 	if (ret) {
> 		errno = ret;
> 		goto err;
> 	}
> ...
> 	return NULL;
>
>
> <back to main>
>
> 	engines = discover_engines(pmu_device);
> 	if (!engines) {
> 		fprintf(stderr,
> 			"Failed to detect engines! (%s)\n(Kernel 4.16 or newer is required 
> for i915 PMU support.)\n",
> 			strerror(errno));
> 		ret = EXIT_FAILURE;
> 		goto err_engines;
> 	}
>
> So tools exits with an error message. Maybe I am blind but, again, I 
> don't see a problem?

When realloc fails, it returns NULL. If we assign that NULL value directly to the
original pointer, we lose the only reference to the previously allocated memory.
Since that memory is no longer accessible and cannot be freed, it effectively
becomes a memory leak for the remainder of the program’s execution. The operating
system will reclaim the memory when the program exits, but during the program’s
lifetime, the leak still exists. After the failure, execution jumps to err_engines,
which skips the cleanup normally performed by free_engines(engines)

-- 
Best regards,
Sebastian