Re: [PATCH 4/6] realpath: let --relative-to default to --relative-base

[Eric Blake]

On 03/14/2012 03:38 PM, Pádraig Brady wrote:
> I wonder is it worth short circuiting some later code
> when relative_to == relative_base, like:
> 
> diff --git a/src/realpath.c b/src/realpath.c
> index 7834c62..c3f5809 100644
> --- a/src/realpath.c
> +++ b/src/realpath.c
> @@ -183,7 +183,8 @@ relpath (const char *can_fname)
>        if (can_relative_base)
>          {
>            if (!path_prefix (can_relative_base, can_fname)
> -              || !path_prefix (can_relative_base, can_relative_to))
> +              || ((can_relative_base != can_relative_to)
> +                  && !path_prefix (can_relative_base, can_relative_to)))

Actually, path_prefix (can_relative_base, can_relative_to) is constant,
and can be factored out of relpath() into main().  I decided to keep
patch 4 as-is, and pushed this optimization as a separate patch; 1-4
plus cleanup is now pushed, leaving just 5 and 6 for any discussion
(I'll push 5 in isolation later this week if we don't have any more
feedback).

From 33ff06ea34730f290e57f3712c384db88096fc44 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 14 Mar 2012 13:42:59 -0600
Subject: [PATCH] realpath: optimize --relative-base usage

There is no need to recompute for every path being visited whether
the base is a prefix of the relative location.

* src/realpath.c (relpath): Hoist base check...
(main): ...here.
Based on a suggestion by Pádraig Brady.
---
 src/realpath.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/realpath.c b/src/realpath.c
index f95535b..206f800 100644
--- a/src/realpath.c
+++ b/src/realpath.c
@@ -179,12 +179,8 @@ relpath (const char *can_fname)
   if (can_relative_to)
     {
       /* Enforce --relative-base.  */
-      if (can_relative_base)
-        {
-          if (!path_prefix (can_relative_base, can_fname)
-              || !path_prefix (can_relative_base, can_relative_to))
-            return false;
-        }
+      if (can_relative_base && !path_prefix (can_relative_base, can_fname))
+        return false;

       /* Skip the prefix common to --relative-to and path.  */
       int common_index = path_common_prefix (can_relative_to, can_fname);
@@ -341,13 +337,25 @@ main (int argc, char **argv)
       if (need_dir && !isdir (can_relative_to))
         error (EXIT_FAILURE, ENOTDIR, "%s", quote (relative_to));
     }
-  if (relative_base)
+  if (relative_base == relative_to)
+    can_relative_base = can_relative_to;
+  else if (relative_base)
     {
-      can_relative_base = realpath_canon (relative_base, can_mode);
-      if (!can_relative_base)
+      char *base = realpath_canon (relative_base, can_mode);
+      if (!base)
         error (EXIT_FAILURE, errno, "%s", quote (relative_base));
-      if (need_dir && !isdir (can_relative_base))
+      if (need_dir && !isdir (base))
         error (EXIT_FAILURE, ENOTDIR, "%s", quote (relative_base));
+      /* --relative-to is a no-op if it does not have --relative-base
+           as a prefix */
+      if (path_prefix (base, can_relative_to))
+        can_relative_base = base;
+      else
+        {
+          free (base);
+          can_relative_base = can_relative_to;
+          can_relative_to = NULL;
+        }
     }

   for (; optind < argc; ++optind)
-- 
1.7.7.6



-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature