Re: [PATCH] attr: fix off-by-one directory component length calculation

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Jan 16, 2013 at 08:08:03AM +0700, Duy Nguyen wrote:
> Actually I'd like to remove that function.

This is what I had in mind:

-- 8< --
Subject: [PATCH] attr: avoid calling find_basename() twice per path

find_basename() is only used inside collect_all_attrs(), called once
in prepare_attr_stack, then again after prepare_attr_stack()
returns. Both calls return exact same value. Reorder the code to do it
once.

While at it, make use of "pathlen" to stop searching early if
possible.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 attr.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/attr.c b/attr.c
index cfc6748..04cb9a0 100644
--- a/attr.c
+++ b/attr.c
@@ -564,32 +564,12 @@ static void bootstrap_attr_stack(void)
 	attr_stack = elem;
 }
 
-static const char *find_basename(const char *path)
-{
-	const char *cp, *last_slash = NULL;
-
-	for (cp = path; *cp; cp++) {
-		if (*cp == '/' && cp[1])
-			last_slash = cp;
-	}
-	return last_slash ? last_slash + 1 : path;
-}
-
-static void prepare_attr_stack(const char *path)
+static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
-	int dirlen, len;
+	int len;
 	const char *cp;
 
-	dirlen = find_basename(path) - path;
-
-	/*
-	 * find_basename() includes the trailing slash, but we do
-	 * _not_ want it.
-	 */
-	if (dirlen)
-		dirlen--;
-
 	/*
 	 * At the bottom of the attribute stack is the built-in
 	 * set of attribute definitions, followed by the contents
@@ -769,15 +749,27 @@ static int macroexpand_one(int attr_nr, int rem)
 static void collect_all_attrs(const char *path)
 {
 	struct attr_stack *stk;
-	int i, pathlen, rem;
-	const char *basename;
+	int i, pathlen, rem, dirlen = 0;
+	const char *basename = path, *cp;
 
-	prepare_attr_stack(path);
+	pathlen = strlen(path);
+
+	/*
+	 * This loop is similar to strrchr(path, '/') except that the
+	 * trailing slash is skipped.
+	 */
+	for (cp = path + pathlen - 2; cp >= path; cp--) {
+		if (*cp == '/') {
+			basename = cp + 1;
+			dirlen = cp - path;
+			break;
+		}
+	}
+
+	prepare_attr_stack(path, dirlen);
 	for (i = 0; i < attr_nr; i++)
 		check_all_attr[i].value = ATTR__UNKNOWN;
 
-	basename = find_basename(path);
-	pathlen = strlen(path);
 	rem = attr_nr;
 	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
 		rem = fill(path, pathlen, basename, stk, rem);
-- 
1.8.0.rc3.18.g0d9b108

-- 8< --