[Toybox] [PATCH] mkpathat: return EEXIST only if MKPATHAT_MKLAST is given

[Rob Landley]

On 2/7/21 10:07 AM, Yi-Yo Chiang via Toybox wrote:
> There was a regression:
> 
> $ mkdir a
> $ touch a/b

I just tried that and it worked with no error?

> Calling mkpath("a/b") would return an error with EEXIST errno. This is
> because "a/b" is a regular file. However mkpath() should only create the
> leading directories of "a/b", which is "a/", which shouldn't be an error
> because attempting to create an already existed directory shouldn't fail.
> 
> ---
>  lib/lib.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/lib.c b/lib/lib.c
> index dfa4499f..76726931 100644
> --- a/lib/lib.c
> +++ b/lib/lib.c
> @@ -177,7 +177,8 @@ int mkpathat(int atfd, char *dir, mode_t lastmode, int flags)
>    // not-a-directory along the way, but the last one we must explicitly
>    // test for. Might as well do it up front.
>  
> -  if (!fstatat(atfd, dir, &buf, 0) && !S_ISDIR(buf.st_mode)) {
> +  if (flags&MKPATHAT_MKLAST && !fstatat(atfd, dir, &buf, 0)
> +      && !S_ISDIR(buf.st_mode)) {
>      errno = EEXIST;
>      return 1;
>    }

For reference, a test that demonstrates the failure is:

  $ ./toybox install -D README a/b
  install: -D 'a/b': File exists

And yes, I need to create and populate an install.tests, but there's a
structural problem ("make install" is an existing make target so can't be used
for the install command, which also knocks out test_install...) It's on the todo
list.. :)

Committed a slightly different fix, with test. (If we successfully statted the
target and don't mind that it's a file, return success immediately...)

Thanks,

Rob