<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 1, 2021 at 3:00 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 8/31/21 11:16 AM, enh via Toybox wrote:<br>
> toys/posix/tar.c:821:7: error: ignoring return value of<br>
> function declared with 'warn_unused_result' attribute<br>
> write(sefd, 0, 0);<br>
> ^~~~~ ~~~~~~~~~~<br>
> ---<br>
> toys/posix/tar.c | 2 +-<br>
> 1 file changed, 1 insertion(+), 1 deletion(-)<br>
<br>
Yeah, I thought about that when I wrote it, but it seems like the correct thing<br>
to do on failure here _is_ nothing? We already warned if the open() or the<br>
write() of the xattr data failed, this is just telling it to reset after the<br>
initial write() to set it displayed any error and set exitval.<br></blockquote><div><br></div><div>to me, if the open() succeeded but the write() failed --- we're in weird uncharted "shouldn't happen" territory, and probably want to know?</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">
I thought about typecasting it to (void) but decided to wait for a complaint<br>
because A)<br>
"CROSS_COMPILE=~/android/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/bin/x86_64-linux-android-<br>
make distclean defconfig toybox" didn't warn about it (obviously -Wall doesn't<br>
mean all, that would be silly), </blockquote><div><br></div><div>(i used to find that annoying, but since my team has to update the compiler for Android, i do now see the flip side of it: that "true" -Wall would mean you could never update the compiler. especially in a -Werror world. we have enough work just dealing with the new warnings that _do_ get added to -Wall! and any given warning's false positive rate can be highly dependent on the specific codebase, and i don't get the feeling compiler writers think about scaling to "can i rebuild all of Debian?" or even just "can i rebuild all of Android?".)</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">B) that meant I couldn't be sure the typecast is<br>
still allowed to squelch it. (Compiler writers are notoriously crotchety about<br>
simple ways to stop them from insisting they know better than the person writing<br>
the program. Well, the gcc guys were.)<br>
<br>
I didn't use xwrite() here because do we really want the program to exit? I<br>
dunno what the potential failure modes here are because "man setfscreatecon"<br>
says on error it returns -1 and DOES NOT MENTION errno. (Bravo. The more I read<br>
of libselinux the less actual engineering competence seems to have gone into it.)<br>
<br>
But I know that _my_ code:<br>
<br>
i = strlen(pp);<br>
sefd = xopen("/proc/self/attr/fscreate", O_WRONLY|WARN_ONLY);<br>
if (sefd==-1 || i!=write(sefd, pp, i))<br>
perror_msg("setfscreatecon %s", pp);<br>
<br>
Didn't close sefd and set it back to -1, it let the exit path to do that. (Since<br>
it was already going to.) Meaning the initial write would warn but not exit, and<br>
then this would exit trying to reset what we never had permission to set,<br>
instead of continuing on to extract what it can.<br></blockquote><div><br></div><div>i'd have assumed (as you can tell from my patch :-) ) that "you asked me to set the selinux labels but i couldn't" == error, not warning?</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">
Although the tar code can survive fd = -1 (when it has selinux data to deploy<br>
it'll warn and move on, when the tarball hasn't got an selinux entry for the<br>
file specifying --selinux is a NOP) it presumably won't ever get it because<br>
"/proc/self/attr/fscreate" seems to always be there (since commit 1b556412becc8<br>
in 2003, which went in circa 2.5.60), even when selinux isn't enabled. I'm not<br>
sure under which circumstances trying to read from or write to it produces<br>
errors because fs/proc/base.c has this horrible LSM_DIR_OPS() macro that's<br>
trying to be template instantiation in C (I have yet to find any PART of selinux<br>
implementation that isn't disgusting) and I lost interest in reading further.<br>
<br>
But on my devuan system without selinux enabled:<br>
<br>
$ cat /proc/self/attr/fscreate<br>
cat: /proc/self/attr/fscreate: Invalid argument<br>
<br>
So I _think_ your patch changes "tar x --selinux" behavior to exit instead of<br>
spamming warnings when run on a system without sexlinux support.<br></blockquote><div><br></div><div>seems like a feature to me? don't say --selinux if you don't actually care?</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">
Does the (void)write(...); typecast still shut up the warning I can't reproduce?<br></blockquote><div><br></div><div>computer says "yes":</div><div><br></div><div>#### build completed successfully (6 seconds) ####<br></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">
Rob<br>
<br>
P.S. gcc has somehow managed to get worse. A dynamically linked bionic toybox says:<br>
<br>
$ ldd ./toybox<br>
./toybox: error while loading shared libraries:<br>
/usr/lib/x86_64-linux-gnu/libm.so: invalid ELF header<br>
<br>
Because /usr/lib/x86_64-linux-gnu/libm.so is a linker script. Bra-filesystem<br>
checking-vo. But hey, it built without warnings which is what I was trying to<br>
check. An in the absence of the string "bionic" in it, there's at least<br>
"note.android" to confirm the binary came from the android NDK toolchain. Or<br>
just V=1 so I can see the invocations go by with the right toolchain path...<br>
_______________________________________________<br>
Toybox mailing list<br>
<a href="mailto:Toybox@lists.landley.net" target="_blank">Toybox@lists.landley.net</a><br>
<a href="http://lists.landley.net/listinfo.cgi/toybox-landley.net" rel="noreferrer" target="_blank">http://lists.landley.net/listinfo.cgi/toybox-landley.net</a><br>
</blockquote></div></div>