On Sat, May 16, 2015 at 8:32 PM, enh <[email protected]> wrote: > > > On Sat, May 16, 2015 at 3:23 PM, Rob Landley <[email protected]> wrote: > >> On 05/16/2015 03:43 PM, enh wrote: >> > actually, this is the one outstanding patch you _shouldn't_ apply --- i >> > had to revert the switch to ls late last night after it broke a >> > surprising amount of infrastructure. >> >> Yeah, I was kinda surprised. I applied your first "add selinux" patch >> verbatim, but it builds with a warning under glibc. > > > that's not "under glibc" so much as "without selinux". this seems to fix > that: > > diff --git a/lib/portability.h b/lib/portability.h > index aa1ee48..4c4a6e7 100644 > --- a/lib/portability.h > +++ b/lib/portability.h > @@ -251,7 +251,8 @@ pid_t xfork(void); > #include <selinux/selinux.h> > #else > #define is_selinux_enabled() 0 > -int getcon(void* con); > +int getcon(void *con); > +int lgetfilecon(const char *path, char **con); > #endif > > #if CFG_TOYBOX_SMACK > > > >> I have pending stuff >> to fix in it, I posted my notes from my first look at it... >> >> > (the toolbox ls didn't columnate, >> >> I think I fixed that one here: >> >> >> https://github.com/landley/toybox/commit/b18c7e8a59e61b8ec49e2d0f8cb2dda19b8f6a82 >> > > no, *toolbox* didn't. toybox ls was actually more compatible when -C > didn't work properly :-) > > >> > and "adb shell ..." creates a pty on >> > the remote side, so toybox ls sees that and defaults to -C rather than >> > -1. callers can't use -1 to say what they want, because toolbox ls >> > didn't support that, and even if we add it today they still need to deal >> > with a mix of old and new devices. >> >> In theory it should do the ANSI size probe to talk to your xterm. You >> could also have "adb shell" export COLUMNS=1 to force -1 mode? >> >> (I'm not sure what the desired behavior is here in "given x, toybox >> should do y" terms. Is "adb shell" being used to run scripts? Provide an >> interactive terminal? Dunno the use case.) > > > "adb shell" is interactive. "adb shell ..." isn't. > > >> > similarly, the pty creation is on the >> > adbd side, so although i'll fix that to be like ssh(1), that will only >> > help new devices. >> >> I'm confused: you get toybox when you upgrade to android m, what >> infrastructure is _not_ upgraded when you do that, causing the version >> skew? > > > test infrastructure. IDEs. that kind of thing. > > >> >> > i suspect we'll have to disable the isatty test in >> > ls_main for the next ten years. >> >> In theory isatty() can also be taught "if the terminal size is 0 by 0 >> when you query it, it's not real". >> >> It's easy enough to make a kconfig switch so the behavior defaults to -1 >> instead of -C even when you have a terminal. (Or the test can be if >> !CFG_TOYBOX_ANDROID...) I'd just like to understand th problem space a >> little better first? > > > oh, me too. i almost didn't write anything but thought that would just > lead to questions, so i wrote the parenthetical explanation of why i'd had > to revert in the hopes that would waste _less_ time. oops. > > short version: toybox is doing nothing wrong, i'm going to see how much > infrastructure i can fix (if i can fix ddmlib, IDEs and the CTS stuff both > just use that), and i'll come back with a specific patch if there's > something i can't fix. > > >> >> > there's also code that parses the >> > toolbox ls -l format, which doesn't have the "nlink" column, so i'll >> > need to find and fix those regular expressions. but first i need to talk >> > to the ddmlib folks to see why they're parsing ls output at all, when >> > using the adb sync protocol queries would seem like a better idea...) >> >> We can add a flag to switch off that column, but given that posix is the >> one that said to have that column: >> >> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/ls.html >> >> That may not be the ideal solution... > > > indeed, and i suspect this is the easiest thing to fix. something like > (?:\s+\d+)? should suffice. > > >> >> > but there's plenty of other stuff like the hwclock fix and the find >> > -inum addition and the xxd for pending to keep you busy, >> >> I'm trying to catch up, but I get on a plane back to japan tomorrow >> afternoon. >> >> (Also, I'm supposed to port $DAYJOB's kernel to smp for sh2 (which is >> fun because sh2 hasn't got an an atomic test-and-set so we may need to >> change the vhdl again), and add dma to our drivers now the dma engine >> can actually generate a completion interrupt, and finish forward porting >> the the kernel to 4.0 (the ethernet driver uses obsolete infrastructure >> to fake hardware timestamps and it broke for the third time between 3.19 >> and 4.0, I need to understand it so I can rip it out. Plus I spent half >> the week unsuccessfully trying to make the h8 port's updated elf2flt >> repo work with my last gplv2 release binutils 2.17, and I'm _still_ >> learning how bits of linking work...) >> >> > plus -- as you >> > said -- there's smack for the flight :-) (rather than make things more >> > confusing and risk slowing things down even further, i'm waiting until >> > the smack changes are in before i worry about SELinux. what i've seen >> > looks plausible though so i'm not anticipating problems.) >> >> There's a first pass review of that already posted, working on some of >> that today... >> >> > btw, on the subject of testing --- i noticed that a command that SEGVs >> > gets a "PASS". i've been meaning to fix that and send a patch, but since >> > it's been two weeks since i noticed already, i should at least report >> it... >> >> Huh. It shouldn't. (Unless it was expected to produce no output? Output >> to stderr can get ignored, and the return value's only meaningful if you >> explicitly test for it ala "command && echo yes"...) >> >> Where did you notice this? (Context?) > > > in my new tests for xxd. at one point xxd was segfaulting but the test > would say PASS. > > checking again, it looks like it's just this test: > > testing "xxd file2" "xxd file2" "" "" "" > > (file2 is an empty file, and the test is to check that we don't output an > address until we have some data.) > > something like this removes the gotcha: > > diff --git a/scripts/runtest.sh b/scripts/runtest.sh > index 8da1089..b9302b0 100644 > --- a/scripts/runtest.sh > +++ b/scripts/runtest.sh > @@ -86,6 +86,11 @@ testing() > echo -ne "$5" | eval "$2" > actual > RETVAL=$? > > + if [ $RETVAL -gt 128 ] && [ $RETVAL -lt 255 ] > + then > + echo "exited with signal (or returned $RETVAL)" >> actual > + fi > + > cmp expected actual > /dev/null 2>&1 > if [ $? -ne 0 ] > then >
ping on this test infrastructure problem since it prevented anyone from adding a test for the ls segfault... > >> Rob >> > > > > -- > Elliott Hughes - http://who/enh - http://jessies.org/~enh/ > Android native code/tools questions? Mail me/drop by/add me as a reviewer. > -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer.
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
