Re: [PATCH i-g-t 01/12] lib/igt_drm_fdinfo: Stop passing key twice

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Fri, Apr 05, 2024 at 01:00:45AM -0500, Lucas De Marchi wrote:
>Change strstartswith() so it also returns the length, which then can be
>used inside the branch when it matches. A good compiler shall optimize
>out the strlen call since the argument is a constant literal.
>
>With this, the find_kv() is not needed anymore and the difference with
>regard the other branches can be summarized with a new ignore_space()
>helper and the fact it matches the entire key by appending the ':'. The
>helper is added on top of the file so it can be reused later.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

LGTM,

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

>---
> lib/igt_drm_fdinfo.c | 87 +++++++++++++++++++++++---------------------
> 1 file changed, 45 insertions(+), 42 deletions(-)
>
>diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>index 18dbb5d0b..bfc946f24 100644
>--- a/lib/igt_drm_fdinfo.c
>+++ b/lib/igt_drm_fdinfo.c
>@@ -53,6 +53,14 @@ static size_t read_fdinfo(char *buf, const size_t sz, int at, const char *name)
> 	return count > 0 ? count : 0;
> }
>
>+static const char *ignore_space(const char *s)
>+{
>+	for (; *s && isspace(*s); s++)
>+		;
>+
>+	return s;
>+}
>+
> static int parse_engine(char *line, struct drm_client_fdinfo *info,
> 			size_t prefix_len,
> 			const char **name_map, unsigned int map_entries,
>@@ -104,23 +112,6 @@ static int parse_engine(char *line, struct drm_client_fdinfo *info,
> 	return found;
> }
>
>-static const char *find_kv(const char *buf, const char *key, size_t keylen)
>-{
>-	const char *p;
>-
>-	if (strncmp(buf, key, keylen))
>-		return NULL;
>-
>-	p = buf + keylen;
>-	if (*p != ':')
>-		return NULL;
>-
>-	for (p++; *p && isspace(*p); p++)
>-		;
>-
>-	return *p ? p : NULL;
>-}
>-
> static int parse_region(char *line, struct drm_client_fdinfo *info,
> 			size_t prefix_len,
> 			const char **region_map, unsigned int region_entries,
>@@ -205,6 +196,11 @@ out:
> 		}							\
> 	} while (0)
>
>+#define strstartswith(a, b, plen__) ({					\
>+		*plen__ = strlen(b);					\
>+		strncmp(a, b, *plen__) == 0;				\
>+})
>+
> unsigned int
> __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> 		       const char **name_map, unsigned int map_entries,
>@@ -222,31 +218,39 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
>
> 	while ((l = strtok_r(_buf, "\n", &ctx))) {
> 		uint64_t val = 0;
>+		size_t keylen;
> 		const char *v;
> 		int idx;
>
> 		_buf = NULL;
>
>-		if ((v = find_kv(l, "drm-driver", strlen("drm-driver")))) {
>-			strncpy(info->driver, v, sizeof(info->driver) - 1);
>-			good++;
>-		}  else if ((v = find_kv(l, "drm-client-id",
>-					 strlen("drm-client-id")))) {
>-			info->id = atol(v);
>-			good++;
>-		} else if ((v = find_kv(l, "drm-pdev", strlen("drm-pdev")))) {
>-			/* optional */
>+		if (strstartswith(l, "drm-driver:", &keylen)) {
>+			v = l + keylen;
>+			v = ignore_space(v);
>+			if (*v) {
>+				strncpy(info->driver, v, sizeof(info->driver) - 1);
>+				good++;
>+			}
>+		}  else if (strstartswith(l, "drm-client-id:", &keylen)) {
>+			v = l + keylen;
>+			v = ignore_space(v);
>+			if (*v) {
>+				info->id = atol(v);
>+				good++;
>+			}
>+		} else if (strstartswith(l, "drm-pdev:", &keylen)) {
>+			v = l + keylen;
>+			v = ignore_space(v);
> 			strncpy(info->pdev, v, sizeof(info->pdev) - 1);
>-		} else if (!strncmp(l, "drm-engine-capacity-", 20)) {
>-			idx = parse_engine(l, info,
>-					   strlen("drm-engine-capacity-"),
>+		} else if (strstartswith(l, "drm-engine-capacity-", &keylen)) {
>+			idx = parse_engine(l, info, keylen,
> 					   name_map, map_entries, &val);
> 			if (idx >= 0) {
> 				info->capacity[idx] = val;
> 				num_capacity++;
> 			}
>-		} else if (!strncmp(l, "drm-engine-", 11)) {
>-			idx = parse_engine(l, info, strlen("drm-engine-"),
>+		} else if (strstartswith(l, "drm-engine-", &keylen)) {
>+			idx = parse_engine(l, info, keylen,
> 					   name_map, map_entries, &val);
> 			if (idx >= 0) {
> 				if (!info->capacity[idx])
>@@ -256,25 +260,24 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> 				if (idx > info->last_engine_index)
> 					info->last_engine_index = idx;
> 			}
>-		} else if (!strncmp(l, "drm-total-", 10)) {
>-			idx = parse_region(l, info, strlen("drm-total-"),
>+		} else if (strstartswith(l, "drm-total-", &keylen)) {
>+			idx = parse_region(l, info, keylen,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, total, val);
>-		} else if (!strncmp(l, "drm-shared-", 11)) {
>-			idx = parse_region(l, info, strlen("drm-shared-"),
>+		} else if (strstartswith(l, "drm-shared-", &keylen)) {
>+			idx = parse_region(l, info, keylen,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, shared, val);
>-
>-		} else if (!strncmp(l, "drm-resident-", 13)) {
>-			idx = parse_region(l, info, strlen("drm-resident-"),
>+		} else if (strstartswith(l, "drm-resident-", &keylen)) {
>+			idx = parse_region(l, info, keylen,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, resident, val);
>-		} else if (!strncmp(l, "drm-purgeable-", 14)) {
>-			idx = parse_region(l, info, strlen("drm-purgeable-"),
>+		} else if (strstartswith(l, "drm-purgeable-", &keylen)) {
>+			idx = parse_region(l, info, keylen,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, purgeable, val);
>-		} else if (!strncmp(l, "drm-active-", 11)) {
>-			idx = parse_region(l, info, strlen("drm-active-"),
>+		} else if (strstartswith(l, "drm-active-", &keylen)) {
>+			idx = parse_region(l, info, keylen,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, active, val);
> 		}
>-- 
>2.44.0
>