[Toybox] [PATCH] Fix mv with trailing slash in source.

Rob Landley rob at landley.net
Mon Jun 24 11:35:12 PDT 2019


On 6/22/19 7:37 AM, Denys Nykula wrote:
> Leaving the Makepost address because somebody abused my email provider
> and got Mozilla and GitHub really unhappy.
> ---
>>From 91c5ba7d250188afa5b40631b15bc3a778691288 Mon Sep 17 00:00:00 2001
> From: Denys Nykula <nykula at ukr.net>
> Date: Sat, 22 Jun 2019 14:57:59 +0300
> Subject: [PATCH] Fix mv with trailing slash in source.
> 
> Press tab, have bash complete dir name with a slash, notice musl
> rename() dislikes that. Replace trailing slash in the cp loop with a
> null character, if the command name is mv. Add the slash back if an
> error occurs.

A) mv "" "" could segfault because you skip the null terminator before checking.
(On Linux it'll remove either the _next_ argument's trailing slash or the first
environment variable's.)

B) Doesn't handle multiple trailing slashes ala mv abc// def

C) this might as well be next to the rename() call...

Ah, I see. Misleading discription. It's not musl rename(), it's that
getbasename() doesn't do what basename() does (intentionaly, but here the other
behavior is what's needed), specifically strace says it's trying to:

  rename("dir/dir2/", "./")

and it's the _second_ part that's screwing it up. According to strace the
gnu/dammit one is doing rename("dir/dir2/", "./dir2"); which works fine.

Sigh. What I should probably do is change the getbasename() semantics to return
a strndup() like getdirname() does, and change all the callers to be ok with
that. Then mv should just do the right thing without this change...

Rob



More information about the Toybox mailing list