Page MenuHomePhabricator

ecore_drm2: fix seat matching when checking for a device with a null seat
ClosedPublic

Authored by n3rdopolis on Dec 26 2017, 9:53 PM.

Details

Summary

Check to make sure that the seat from the matching device is still null.

Test Plan

Make sure that devices are not being rejected while on seat1. This depends on another patch

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
n3rdopolis created this revision.Dec 26 2017, 9:53 PM
ManMower added a subscriber: ManMower.

What code path actually ends up here with !seat? the only two call sites I'm aware of aren't capable of passing NULL.

I think we could put an EINA_SAFETY check at the top of ecore_drm2_device_open for seat, and just not allow it to be NULL ever.

Do you mean
EINA_SAFETY_ON_NULL_RETURN(seat);
?

In this case I think
EINA_SAFETY_ON_NULL_RETURN_VAL(seat, NULL);

But that takes us to this:
!seat is always false, and probably always has been

so the conditional in your change is always false, and we never filter out seat0 tagged devices from other seats?

Hm... ...I don't know what the second if statement was for...
...at first I thought that in case if dseat was NULL (in case if the seat attribute was not set in udev)

...but looking that again, I just realized the order of dseat is flipped in the second one... ...AND dseat is defaulted to seat0 it is null a few lines before, to account for udev returning null on the devices seat attribute...
so it seems that I guess if

_drm2_device_find

was called without the seat argument as NULL, assume that the caller meant the default "seat0"

I thought that maybe there was the possibility that calling _drm2_device_find with the second argument as NULL was valid in some way... (since it's pretty similar in ecore_drm_device_find in non v2 ecore_drm_device.c )

...if not I'll change it

n3rdopolis updated this revision to Diff 13658.Jan 10 2018, 4:27 PM

Add EINA_SAFETY_ON_NULL_RETURN_VAL(seat, NULL) check

I updated the diff
I don't know if ...is it valid for the first argument of ecore_drm2_device_open to be NULL? (assuming it is running on seat0 , otherwise it won't work)
because if it is this might be an API break compared to the last one I submitted...

devilhorns requested changes to this revision.Jan 11 2018, 5:53 AM
devilhorns added inline comments.
src/lib/ecore_drm2/ecore_drm2_device.c
97

Please move this line to be below the variable declarations

This revision now requires changes to proceed.Jan 11 2018, 5:53 AM
n3rdopolis updated this revision to Diff 13667.Jan 11 2018, 4:44 PM
devilhorns accepted this revision.Jan 18 2018, 5:15 AM

This looks ok to me now :) @ManMower thoughts ??

This revision is now accepted and ready to land.Jan 18 2018, 5:15 AM
This revision was automatically updated to reflect the committed changes.