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