Re: [PATCH i-g-t 1/3] lib/igt_sysfs: Add xe_sysfs_gt_has_attr to check attribute existence

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

On 7/31/2024 8:00 PM, Kamil Konieczny wrote:
> Hi Marcin,
> On 2024-07-30 at 18:42:30 +0200, Marcin Bernatowicz wrote:
>> Introduce xe_sysfs_gt_has_attr to verify if a specified attribute
>> exists within the sysfs gt directory of a device.
>>
>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>> ---
>>   lib/igt_sysfs.c | 25 +++++++++++++++++++++++++
>>   lib/igt_sysfs.h |  1 +
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
>> index 550472d81..bcfd9fad1 100644
>> --- a/lib/igt_sysfs.c
>> +++ b/lib/igt_sysfs.c
>> @@ -265,6 +265,31 @@ int xe_sysfs_gt_open(int xe_device, int gt)
>>   	return open(path, O_RDONLY);
>>   }
>>   
>> +/**
>> + * xe_sysfs_gt_has_attr:
>> + * @xe_device: fd of the device
>> + * @gt: gt number
>> + * @attr: attr inside sysfs gt dir that needs to be checked for existence
>> + *
>> + * This checks if specified attr exists in device sysfs gt directory.
>> + *
>> + * Returns:
>> + * true if attr exists in sysfs, false otherwise.
>> + */
>> +bool xe_sysfs_gt_has_attr(int xe_device, int gt, const char *attr)
>> +{
>> +	bool has_attr;
>> +	int gt_fd;
>> +
>> +	gt_fd = xe_sysfs_gt_open(xe_device, gt);
>> +	if (gt_fd < 0)
>> +		return false;
>> +
>> +	has_attr = igt_sysfs_has_attr(gt_fd, attr);
> 
> Did you check that this will not assert on some unexpected
> boundary conditions?

igt_sysfs_has_attr does not assert

> 
>> +	close(gt_fd);
> 
> Add newline before return here, with this:

Ok
> 
> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> 
>> +	return has_attr;
>> +}
>> +
>>   static const char *xe_engine_class_to_str(__u16 class)
>>   {
>>   	static const char * const str[] = {
>> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
>> index 5d050c786..b0288fe36 100644
>> --- a/lib/igt_sysfs.h
>> +++ b/lib/igt_sysfs.h
>> @@ -168,6 +168,7 @@ void igt_sysfs_engines(int xe, int engines, const char **property,
>>   
>>   char *xe_sysfs_gt_path(int xe_device, int gt, char *path, int pathlen);
>>   int xe_sysfs_gt_open(int xe_device, int gt);
>> +bool xe_sysfs_gt_has_attr(int xe_device, int gt, const char *attr);
>>   char *xe_sysfs_tile_path(int xe_device, int tile, char *path, int pathlen);
>>   int xe_sysfs_tile_open(int xe_device, int tile);
>>   int xe_sysfs_get_num_tiles(int xe_device);
>> -- 
>> 2.31.1
>>