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 <[email protected]>
> 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
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to