[i-g-t] igt/sysfs: drop support for passing -1 fd for Intel

Submitted by Jani Nikula on Jan. 11, 2019, 8:57 a.m.

Details

Message ID 20190111085735.10613-1-jani.nikula@intel.com
State Accepted
Commit 71cea1269599f18b6f9a20da1e88f64dcf6e556c
Headers show
Series "igt/sysfs: drop support for passing -1 fd for Intel" ( rev: 1 ) in IGT

Not browsing as part of any series.

Commit Message

Jani Nikula Jan. 11, 2019, 8:57 a.m.
The rabbit hole goes deep in this case, but I couldn't find any place
where we'd still rely on -1 for Intel. Drop the remaining support to
prevent anyone adding new code using this.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 lib/igt_sysfs.c | 50 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 27 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index cce342a057e3..c57e4ae23c5e 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -86,7 +86,7 @@  static int writeN(int fd, const char *buf, int len)
 
 /**
  * igt_sysfs_path:
- * @device: fd of the device (or -1 to default to Intel)
+ * @device: fd of the device
  * @path: buffer to fill with the sysfs path to the device
  * @pathlen: length of @path buffer
  * @idx: optional pointer to store the card index of the opened device
@@ -100,33 +100,29 @@  char *igt_sysfs_path(int device, char *path, int pathlen, int *idx)
 {
 	struct stat st;
 
-	if (device != -1 && (fstat(device, &st) || !S_ISCHR(st.st_mode)))
+	if (device < 0)
+		return NULL;
+
+	if (fstat(device, &st) || !S_ISCHR(st.st_mode))
 		return NULL;
 
 	for (int n = 0; n < 16; n++) {
-		int len = snprintf(path, pathlen, "/sys/class/drm/card%d", n);
-		if (device != -1) {
-			FILE *file;
-			int ret, maj, min;
-
-			sprintf(path + len, "/dev");
-			file = fopen(path, "r");
-			if (!file)
-				continue;
-
-			ret = fscanf(file, "%d:%d", &maj, &min);
-			fclose(file);
-
-			if (ret != 2 ||
-			    major(st.st_rdev) != maj ||
-			    minor(st.st_rdev) != min)
-				continue;
-		} else {
-			/* Bleh. Search for intel */
-			sprintf(path + len, "/error");
-			if (stat(path, &st))
-				continue;
-		}
+		int len, ret, maj, min;
+		FILE *file;
+
+		len = snprintf(path, pathlen, "/sys/class/drm/card%d", n);
+
+		sprintf(path + len, "/dev");
+		file = fopen(path, "r");
+		if (!file)
+			continue;
+
+		ret = fscanf(file, "%d:%d", &maj, &min);
+		fclose(file);
+
+		if (ret != 2 || major(st.st_rdev) != maj ||
+		    minor(st.st_rdev) != min)
+			continue;
 
 		path[len] = '\0';
 		if (idx)
@@ -139,7 +135,7 @@  char *igt_sysfs_path(int device, char *path, int pathlen, int *idx)
 
 /**
  * igt_sysfs_open:
- * @device: fd of the device (or -1 to default to Intel)
+ * @device: fd of the device
  * @idx: optional pointer to store the card index of the opened device
  *
  * This opens the sysfs directory corresponding to device for use
@@ -160,7 +156,7 @@  int igt_sysfs_open(int device, int *idx)
 
 /**
  * igt_sysfs_set_parameters:
- * @device: fd of the device (or -1 to default to Intel)
+ * @device: fd of the device
  * @parameter: the name of the parameter to set
  * @fmt: printf-esque format string
  *

Comments

On Fri, 11 Jan 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> The rabbit hole goes deep in this case, but I couldn't find any place
> where we'd still rely on -1 for Intel. Drop the remaining support to
> prevent anyone adding new code using this.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Pushed with Chris' irc r-b.

BR,
Jani.


> ---
>  lib/igt_sysfs.c | 50 +++++++++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index cce342a057e3..c57e4ae23c5e 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -86,7 +86,7 @@ static int writeN(int fd, const char *buf, int len)
>  
>  /**
>   * igt_sysfs_path:
> - * @device: fd of the device (or -1 to default to Intel)
> + * @device: fd of the device
>   * @path: buffer to fill with the sysfs path to the device
>   * @pathlen: length of @path buffer
>   * @idx: optional pointer to store the card index of the opened device
> @@ -100,33 +100,29 @@ char *igt_sysfs_path(int device, char *path, int pathlen, int *idx)
>  {
>  	struct stat st;
>  
> -	if (device != -1 && (fstat(device, &st) || !S_ISCHR(st.st_mode)))
> +	if (device < 0)
> +		return NULL;
> +
> +	if (fstat(device, &st) || !S_ISCHR(st.st_mode))
>  		return NULL;
>  
>  	for (int n = 0; n < 16; n++) {
> -		int len = snprintf(path, pathlen, "/sys/class/drm/card%d", n);
> -		if (device != -1) {
> -			FILE *file;
> -			int ret, maj, min;
> -
> -			sprintf(path + len, "/dev");
> -			file = fopen(path, "r");
> -			if (!file)
> -				continue;
> -
> -			ret = fscanf(file, "%d:%d", &maj, &min);
> -			fclose(file);
> -
> -			if (ret != 2 ||
> -			    major(st.st_rdev) != maj ||
> -			    minor(st.st_rdev) != min)
> -				continue;
> -		} else {
> -			/* Bleh. Search for intel */
> -			sprintf(path + len, "/error");
> -			if (stat(path, &st))
> -				continue;
> -		}
> +		int len, ret, maj, min;
> +		FILE *file;
> +
> +		len = snprintf(path, pathlen, "/sys/class/drm/card%d", n);
> +
> +		sprintf(path + len, "/dev");
> +		file = fopen(path, "r");
> +		if (!file)
> +			continue;
> +
> +		ret = fscanf(file, "%d:%d", &maj, &min);
> +		fclose(file);
> +
> +		if (ret != 2 || major(st.st_rdev) != maj ||
> +		    minor(st.st_rdev) != min)
> +			continue;
>  
>  		path[len] = '\0';
>  		if (idx)
> @@ -139,7 +135,7 @@ char *igt_sysfs_path(int device, char *path, int pathlen, int *idx)
>  
>  /**
>   * igt_sysfs_open:
> - * @device: fd of the device (or -1 to default to Intel)
> + * @device: fd of the device
>   * @idx: optional pointer to store the card index of the opened device
>   *
>   * This opens the sysfs directory corresponding to device for use
> @@ -160,7 +156,7 @@ int igt_sysfs_open(int device, int *idx)
>  
>  /**
>   * igt_sysfs_set_parameters:
> - * @device: fd of the device (or -1 to default to Intel)
> + * @device: fd of the device
>   * @parameter: the name of the parameter to set
>   * @fmt: printf-esque format string
>   *