<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 8, 2021 at 3:41 PM Rob Landley <<a href="mailto:rob@landley.net">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/7/21 10:07 AM, Yi-Yo Chiang via Toybox wrote:<br>
> There was a regression:<br>
> <br>
> $ mkdir a<br>
> $ touch a/b<br>
<br>
I just tried that and it worked with no error?<br></blockquote><div><br></div><div>I should've been more clear. I mean after doing</div><div>$ touch "a/b"</div><div>"a/b" is now a regular file, and then subsequent calls of mkpath("a/b") would return 1 with errno EEXIST.</div><div>mkdir or touch itself doesn't error. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Calling mkpath("a/b") would return an error with EEXIST errno. This is<br>
> because "a/b" is a regular file. However mkpath() should only create the<br>
> leading directories of "a/b", which is "a/", which shouldn't be an error<br>
> because attempting to create an already existed directory shouldn't fail.<br>
> <br>
> ---<br>
> lib/lib.c | 3 ++-<br>
> 1 file changed, 2 insertions(+), 1 deletion(-)<br>
> <br>
> diff --git a/lib/lib.c b/lib/lib.c<br>
> index dfa4499f..76726931 100644<br>
> --- a/lib/lib.c<br>
> +++ b/lib/lib.c<br>
> @@ -177,7 +177,8 @@ int mkpathat(int atfd, char *dir, mode_t lastmode, int flags)<br>
> // not-a-directory along the way, but the last one we must explicitly<br>
> // test for. Might as well do it up front.<br>
> <br>
> - if (!fstatat(atfd, dir, &buf, 0) && !S_ISDIR(buf.st_mode)) {<br>
> + if (flags&MKPATHAT_MKLAST && !fstatat(atfd, dir, &buf, 0)<br>
> + && !S_ISDIR(buf.st_mode)) {<br>
> errno = EEXIST;<br>
> return 1;<br>
> }<br>
<br>
For reference, a test that demonstrates the failure is:<br>
<br>
$ ./toybox install -D README a/b<br>
install: -D 'a/b': File exists<br>
<br></blockquote><div><br></div><div>Another repro step is with cpio. I (well my coworker really) were doing something like this:</div><div>$ mkdir a</div><div>$ touch a/b</div><div>$ find a | cpio -o >a.cpio</div><div>$ cpio -id <a.cpio</div><div>cpio: a: File exists<br>cpio: mkpath 'a/b': File exists </div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
And yes, I need to create and populate an install.tests, but there's a<br>
structural problem ("make install" is an existing make target so can't be used<br>
for the install command, which also knocks out test_install...) It's on the todo<br>
list.. :)<br></blockquote><div><br></div><div>I was hesitant to attach a testcase demonstrated by "cpio", as I don't think cpio is the real cause here. It just happens so cpio were calling mkpath in the correct order.</div><div>If only we had a test case that directly test lib/lib.c itself?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Committed a slightly different fix, with test. (If we successfully statted the<br>
target and don't mind that it's a file, return success immediately...)<br></blockquote><div><br></div><div>Sounds good! If stat() succeeded, then no need to create anything.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
<br>
Rob<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><table width="90%" border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:0px;font-family:"Times New Roman";max-width:348px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td style="padding:0px"><table border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:20px 0px 0px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td valign="top" style="padding:0px 20px 0px 0px;vertical-align:top;border-right:1px solid rgb(213,213,213)"><img src="https://i.imgur.com/eGpkLls.png" width="200" height="64"><br></td><td style="padding:0px 0px 0px 20px"><table border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:0px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:1px 0px 5px;font-size:13px;line-height:13px;color:rgb(56,58,53);font-weight:700">Yi-yo Chiang</td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 5px;font-size:11px;line-height:13px;color:rgb(56,58,53)">Software Engineer</td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 5px;font-size:11px;line-height:13px;color:rgb(56,58,53)"><a href="mailto:yochiang@google.com" target="_blank" class="cremed">yochiang@google.com</a></td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 3px;font-size:11px;line-height:13px;color:rgb(3,112,248)"></td></tr></tbody></table></td></tr></tbody></table></td></tr></tbody></table></div></div></div>