[igt-dev] [PATCH i-g-t 2/4] Introduce device selection API

Zbigniew KempczyƄski zbigniew.kempczynski at intel.com
Thu Oct 24 16:40:31 UTC 2019


On Thu, Oct 24, 2019 at 02:05:13PM +0300, Arkadiusz Hiler wrote:

<cut>

I've taken a quick look and I think this require change:

> +/**
> + * igt_device_filter_set
> + * @filter: filter that should be set globally
> + *
> + * Returns number of filters added to filter array. Can be greater than
> + * 1 if @filters contains more than one filter separated by semicolon.
> + */
> +void igt_device_filter_set(const char *filter)
> +{
> +	if (!is_filter_valid(filter)) {
> +		igt_warn("Invalid filter: %s\n", filter);
> +		return;
> +	}
> +
> +	device_filter = strdup(filter);
> +}

Comment informs function returns number of filters but it returns void
now. What we should also protect is to memleak if user passes different
filters because we don't free previous device_filter string if already
set.

Same in the header file.

> +
> +/**
> + * igt_device_filter_free
> + *
> + * Free the filter that we store internally, effectively insetting it.
> + */
> +void igt_device_filter_free(void)
> +{
> +	if (device_filter != NULL)
> +		free(device_filter);
> +}

I would also add nullyfiing device_filter here.

That's for brief look.

Thanks for reviewing and rewriting patches.

Zbigniew


More information about the igt-dev mailing list