Duy Nguyen <[email protected]> writes:
> OK I get your point now. Something like this?
>
> -- 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
> the same task once. Also avoid strlen() because we knows the length
> after finding basename.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
Yeah, I think this is a nice code reduction, readability improvement
and micro optimization rolled into one.
> attr.c | 45 ++++++++++++++++++---------------------------
> 1 file changed, 18 insertions(+), 27 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index cfc6748..880f862 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,26 @@ 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;
> + const char *basename, *cp, *last_slash = NULL;
> +
> + for (cp = path; *cp; cp++) {
> + if (*cp == '/' && cp[1])
> + last_slash = cp;
> + }
> + pathlen = cp - path;
> + if (last_slash) {
> + basename = last_slash + 1;
> + dirlen = last_slash - path;
> + } else {
> + basename = path;
> + dirlen = 0;
> + }
>
> - prepare_attr_stack(path);
> + 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);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html