Duy Nguyen <pclo...@gmail.com> 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 <pclo...@gmail.com>

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to