[Mesa-dev] [PATCH 1/4] r600_state_common: check NULL return from u_upload_alloc

[Julien Isorce]

1: Ack, thx.
2: u_upload_alloc can fail, i.e. set output param *ptr to NULL, for 2
reasons: alloc fails or map fails. For both there are already a
fprintf/stderr, i.e., in radeon_create_bo and radeon_bo_do_map .
It looks to me that in src/gallium/drivers/ it is a common usage to just
avoid to crash by doing a silent check. But it defers the fprintf to where
the error comes from, i.e. libdrm calls.

Also I think that if drmCommandWriteRead returns -ENOMEM it should be
translated to _mesa_error_no_memory(__func__); at some point. But it seems
it is not possible to use _mesa_error_no_memory in mesa/src/gallium/winsys
(though it is possible from gallium/state_trackers/glx). And it would
require to be in the gl context's thread to call it.

Regards
Julien


On 24 March 2017 at 11:19, Nicolai Hähnle <nhaehnle at gmail.com> wrote:

> I generally like the patches in the series, thanks for that. Two points
> though:
>
> 1. The order of patches in series is usually general code before driver
> code, i.e. the st/cb_bitmap should come first.
>
> 2. I don't like having silent errors, as that could be confusing. In
> places where the error isn't propagated to the application (as it really
> should be...), I think we should have an fprintf to stderr.
>
> Cheers,
> Nicolai
>
> On 24.03.2017 12:08, Julien Isorce wrote:
>
>> Like done in si_state_draw.c::si_draw_vbo
>>
>> Signed-off-by: Julien Isorce <jisorce at oblong.com>
>> ---
>>  src/gallium/drivers/r600/r600_state_common.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/gallium/drivers/r600/r600_state_common.c
>> b/src/gallium/drivers/r600/r600_state_common.c
>> index 6f8279f..cedeb74 100644
>> --- a/src/gallium/drivers/r600/r600_state_common.c
>> +++ b/src/gallium/drivers/r600/r600_state_common.c
>> @@ -1746,6 +1746,10 @@ static void r600_draw_vbo(struct pipe_context
>> *ctx, const struct pipe_draw_info
>>
>>                         u_upload_alloc(ctx->stream_uploader, start,
>> count * 2,
>>                                         256, &out_offset, &out_buffer,
>> &ptr);
>> +                       if (unlikely(!ptr)) {
>> +                               pipe_resource_reference(&ib.buffer,
>> NULL);
>> +                               return;
>> +                       }
>>
>>                         util_shorten_ubyte_elts_to_userptr(
>>                                                 &rctx->b.b, &ib, 0, 0,
>> ib.offset + start, count, ptr);
>>
>>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170324/9a7f2b87/attachment.html>