Re: [Intel-gfx] [PATCH i-g-t 5/8] tests/i915/gem_exec_capture: Check for memory allocation failure

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 22/10/2021 00:40, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> The sysfs file read helper does not actually report any errors if a
> realloc fails. It just silently returns a 'valid' but truncated
> buffer. This then leads to the decode of the buffer failing in random
> ways. So, add a check for ENOMEM being generated during the read.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>   tests/i915/gem_exec_capture.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tests/i915/gem_exec_capture.c b/tests/i915/gem_exec_capture.c
> index e373d24ed..8997125ee 100644
> --- a/tests/i915/gem_exec_capture.c
> +++ b/tests/i915/gem_exec_capture.c
> @@ -131,9 +131,11 @@ static int check_error_state(int dir, struct offset *obj_offsets, int obj_count,
>   	char *error, *str;
>   	int blobs = 0;
>   
> +	errno = 0;
>   	error = igt_sysfs_get(dir, "error");
>   	igt_sysfs_set(dir, "error", "Begone!");
>   	igt_assert(error);
> +	igt_assert(errno != ENOMEM);

igt_sysfs_get:

	len = 64;
...
                 newbuf = realloc(buf, 2*len);

Maybe the problem is doubling goes out of hand. How big are your 
buffers? Perhaps you could improve the library function instead to grow 
less aggressively.

And at the same time perhaps the bug is this:

                 if (igt_debug_on(!newbuf))
                         break;
...
         return buf;

So failures to grow the buffer are ignored, while failure to allocate 
the initial one are not. Perhaps both should return NULL and then 
callers would not be surprised.

Or you think someone relies on this current odd behaviour?

Regards,

Tvrtko

>   	igt_debug("%s\n", error);
>   
>   	/* render ring --- user = 0x00000000 ffffd000 */
>