Re: [Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-03 Thread enh via Toybox
On Mon, Feb 1, 2021 at 8:06 PM Rob Landley wrote: > On 2/1/21 9:53 PM, Yi-yo Chiang wrote: > > There has to be a reason, but I'll defer that question to Elliott.. > > >

Re: [Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-01 Thread Yi-yo Chiang via Toybox
I found the original commit that adds this: https://android.googlesource.com/platform/bionic/+/35778253a5ed71e87a608ca590b63729d9f88567 There is even a link to this mailing list... Can't say I completely follow the discussion, as the thread is long and dated, and I may be missing a lot of

Re: [Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-01 Thread Yi-yo Chiang via Toybox
(forgot to cc Elliott) On Tue, Feb 2, 2021, 12:06 Rob Landley wrote: > On 2/1/21 9:53 PM, Yi-yo Chiang wrote: > > There has to be a reason, but I'll defer that question to Elliott.. > > >

Re: [Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-01 Thread Rob Landley
On 2/1/21 9:53 PM, Yi-yo Chiang wrote: > There has to be a reason, but I'll defer that question to Elliott.. > https://cs.android.com/android/platform/superproject/+/master:bionic/libc/bionic/faccessat.cpp;l=46;drc=50080a29f7327fcd009344844bb9e643b2d6b9c3 > > This line also left me scratching my

Re: [Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-01 Thread Yi-yo Chiang via Toybox
There has to be a reason, but I'll defer that question to Elliott.. https://cs.android.com/android/platform/superproject/+/master:bionic/libc/bionic/faccessat.cpp;l=46;drc=50080a29f7327fcd009344844bb9e643b2d6b9c3 This line also left me scratching my head for half a day.

Re: [Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-01 Thread Rob Landley
On 2/1/21 1:22 PM, Yi-Yo Chiang via Toybox wrote: > -// Files that already don't exist aren't errors for -f, so try a quick > -// unlink now to see if it succeeds or reports that it didn't exist. > -if (FLAG(f) && (!unlink(*s) || errno == ENOENT)) continue; > +// Files that already

[Toybox] [PATCH 1/2] rm: Check file existence with lstat() explicitly if "-f" is specified

2021-02-01 Thread Yi-Yo Chiang via Toybox
Instead of unlink() && check errno, call lstat() explicitly to check file existence if "-f" is specified. There is a regression when if the path to be removed is nonexistence and within a readonly filesystem, then unlink() could set the EROFS errno instead of ENOENT, thus screwing up the output of