[Mesa-dev] [PATCH 1/7] egl/dri2: Avoid sign extension when building modifier

[Daniel Stone]

Hi Eric,

On 6 June 2017 at 18:27, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> On Tuesday, 2017-06-06 18:18:31 +0100, Daniel Stone wrote:
>> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
>> index d31a0bf8e0..7175e827c9 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.c
>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>> @@ -2278,9 +2278,8 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
>>      * will be present in attrs.DMABufPlaneModifiersLo[0] and
>>      * attrs.DMABufPlaneModifiersHi[0] */
>>     if (attrs.DMABufPlaneModifiersLo[0].IsPresent) {
>> -      modifier =
>> -         ((uint64_t) attrs.DMABufPlaneModifiersHi[0].Value << 32) |
>> -         attrs.DMABufPlaneModifiersLo[0].Value;
>> +      modifier = (uint64_t) attrs.DMABufPlaneModifiersHi[0].Value << 32;
>> +      modifier |= (uint64_t) (attrs.DMABufPlaneModifiersLo[0].Value & 0xffffffff);
>
> To be clear, the fix is to cast Lo before OR'ing it, right?
> The rest is just aesthetic?

Yeah, pretty much. A quick test suggests that the cast isn't even
strictly necessary: presumably the promotion (and sign-extension) from
int32 -> uint64 happens before the AND op in (u64 = i32 & 0xffffffff).
I figured being explicit couldn't do any harm anyway.

Cheers,
Daniel