[PATCH i-g-t] tests/device_reset: Skip if no drm devices are preset

[Nirmoy Das]

On 3/12/2024 12:14 PM, Janusz Krzysztofik wrote:
> On Tuesday, 12 March 2024 11:54:32 CET Nirmoy Das wrote:
>> On 3/12/2024 11:43 AM, Janusz Krzysztofik wrote:
>>> Hi Nirmoy,
>>>
>>> On Tuesday, 12 March 2024 11:07:57 CET Nirmoy Das wrote:
>>>> Skip the test if no drm devices are preset.
>>> Did you mean present? (also in the subject line)
>> Right, another typo
>>>> v2: fix a typo.
>>>>
>>>> Closes:https://gitlab.freedesktop.org/drm/intel/-/issues/10087
>>>> Signed-off-by: Nirmoy Das<nirmoy.das at intel.com>
>>>> ---
>>>>    tests/device_reset.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/device_reset.c b/tests/device_reset.c
>>>> index 817cbc285..73531d696 100644
>>>> --- a/tests/device_reset.c
>>>> +++ b/tests/device_reset.c
>>>> @@ -198,7 +198,7 @@ static void init_device_fds(struct device_fds *dev)
>>>>    	 * a device file descriptor open for exit handler use.
>>>>    	 */
>>>>    	dev->fds.dev = __drm_open_driver(DRIVER_ANY);
>>>> -	igt_assert_fd(dev->fds.dev);
>>>> +	igt_require(dev->fds.dev != -1);
>>> What if __drm_open_driver() returns (unlikely) a negative value other than
> -1?
>> __drm_open_driverdoc says: "An open DRM fd or -1 on error" so that won't
> happen.
>
> That won't happen as long as there are no bugs in __drm_open_driver().
>
> If you don't want to skip on (fd < -1) then let's fail, but we should never
> succeed in that unlikely case, I believe.

No, we should take documented return value as a contract and shouldn't 
over worry about the function

breaking it.


Regards,

Nirmoy


>
> Thanks,
> Janusz
>
>>>    
>>> In IGT we used to verify validity of a file descriptor by checking if its
>>> value is not negative, i.e., >= 0 (see e.g. lib/igt_core:igt_assert_fd(),
>>> lib/drmtest.c:drm_open_driver()).  Please follow the same convention.
>> I can do s/igt_assert_fd/igt_require_fd
>>
>> Thanks,
>>
>> Nirmoy
>>
>>> Thanks,
>>> Janusz
>>>
>>>>    	if (is_i915_device(dev->fds.dev))
>>>>    		igt_require_gem(dev->fds.dev);
>>>>    
>>>>
>>>
>>>
>
>
>