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