Bug with :P modifier when current directory is the root

Bug with :P modifier when current directory is the root

[Bart Schaefer]

Debugging in another thread, I added this line to
_canonical_paths_add_paths --

    : curpref is $curpref and :P is $curpref:P

and _complete_debug (with "$PWD" == "/") says

    +_canonical_paths_add_paths:11> : curpref is m and :P is //m

I can't think of any case [*] where the :P modifier should add a double
leading slash.

[*] Except on weird OSs where you refer to auto-mount points with //host/
but that's definitely not the case here.

Does this need an extra test that (here[strlen(here)-1] != '/') or some
such, or are we safe enough this way?  Can zgetcwd() ever return "//" or
"///" etc.?


diff --git a/Src/subst.c b/Src/subst.c
index 2214b3d..d68f16b 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -4336,7 +4336,11 @@ modify(char **str, char **ptr)
 			break;
 		    case 'P':
 			if (*copy != '/') {
-			    copy = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", copy);
+			    char *here = zgetcwd();
+			    if (here[1])
+				copy = zhtricat(metafy(here, -1, META_HEAPDUP), "/", copy);
+			    else
+				copy = dyncat(here, copy);
 			}
 			copy = xsymlink(copy, 1);
 			break;
@@ -4418,7 +4422,11 @@ modify(char **str, char **ptr)
 		    break;
 		case 'P':
 		    if (**str != '/') {
-			*str = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", *str);
+			char *here = zgetcwd();
+			if (here[1])
+			    *str = zhtricat(metafy(here, -1, META_HEAPDUP), "/", *str);
+			else
+			    *str = dyncat(here, *str);
 		    }
 		    *str = xsymlink(*str, 1);
 		    break;

Re: Bug with :P modifier when current directory is the root

[Daniel Shahaf]

Bart Schaefer wrote on Fri, Mar 10, 2017 at 18:25:01 -0800:
> Does this need an extra test that (here[strlen(here)-1] != '/') or some
> such, or are we safe enough this way?  Can zgetcwd() ever return "//" or
> "///" etc.?

zgetcwd() can return "." if the current working directory has been
deleted by another process.

I see zgetcwd() never returns NULL, but can it return an empty string?

> diff --git a/Src/subst.c b/Src/subst.c
> index 2214b3d..d68f16b 100644
> --- a/Src/subst.c
> +++ b/Src/subst.c
> @@ -4336,7 +4336,11 @@ modify(char **str, char **ptr)
>  			break;
>  		    case 'P':
>  			if (*copy != '/') {
> -			    copy = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", copy);
> +			    char *here = zgetcwd();
> +			    if (here[1])
> +				copy = zhtricat(metafy(here, -1, META_HEAPDUP), "/", copy);
> +			    else
> +				copy = dyncat(here, copy);
>  			}
>  			copy = xsymlink(copy, 1);
>  			break;

Re: Bug with :P modifier when current directory is the root

[Bart Schaefer]

On Mar 11,  8:47am, Daniel Shahaf wrote:
}
} zgetcwd() can return "." if the current working directory has been
} deleted by another process.

One of those cases it's annoying to have to waste time on, since it is
so rare and the result will be broken anyway, but I suppose I should
commit it like the below.

diff --git a/Src/subst.c b/Src/subst.c
index 2214b3d..e639c96 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -4336,7 +4336,11 @@ modify(char **str, char **ptr)
 			break;
 		    case 'P':
 			if (*copy != '/') {
-			    copy = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", copy);
+			    char *here = zgetcwd();
+			    if (here[strlen(here)-1] != '/')
+				copy = zhtricat(metafy(here, -1, META_HEAPDUP), "/", copy);
+			    else
+				copy = dyncat(here, copy);
 			}
 			copy = xsymlink(copy, 1);
 			break;
@@ -4418,7 +4422,11 @@ modify(char **str, char **ptr)
 		    break;
 		case 'P':
 		    if (**str != '/') {
-			*str = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP), "/", *str);
+			char *here = zgetcwd();
+			if (here[strlen(here)-1] != '/')
+			    *str = zhtricat(metafy(here, -1, META_HEAPDUP), "/", *str);
+			else
+			    *str = dyncat(here, *str);
 		    }
 		    *str = xsymlink(*str, 1);
 		    break;

Re: Bug with :P modifier when current directory is the root

[Oliver Kiddle]

Bart wrote:
> I can't think of any case [*] where the :P modifier should add a double
> leading slash.
>
> [*] Except on weird OSs where you refer to auto-mount points with //host/
> but that's definitely not the case here.

Preserving of two initial slashes is, I think, in the spec and has long
been reserved to allow "weird OSs" to do odd stuff like specifying
remote hosts. That would imply that :P should preserve two slashes while
three or more should be squeezed down to one. See:
https://jvns.ca/blog/2017/02/08/weird-unix-things-cd/

Oliver

Re: Bug with :P modifier when current directory is the root

[Bart Schaefer]

On Mar 12,  2:58am, Oliver Kiddle wrote:
}
} That would imply that :P should preserve two slashes while
} three or more should be squeezed down to one.

That's an entirely different branch of code and currently follows
whatever xsymlinks() does.  In this thread we're discussing what
happens when the value of the variable does not already begin with
a slash and the current working directory is therefore prepended.