On Sat, Jul 11, 2015 at 12:22 PM, Rob Landley <[email protected]> wrote: > On 07/10/2015 09:25 PM, enh wrote: >> (already submitted as https://android-review.googlesource.com/159035/ >> --- i was under pressure because i didn't notice the problem until i'd >> already broken all the 32-bit builds. as usual i tested on 64-bit. >> i've removed the non-standard basename_r and dirname_r from 64-bit > > Sigh. There's already a basename_r. Well of course there is.
it's worse than that --- there are several. the BSD one isn't the same as the bionic one. (and neither of them are the same as yours.) > It sounds like a good idea, personally I'd submit it to posix rather > than remove it. A better fix would be to move basename_r() to > portability.c and rely on libc to provide it if they have it. (I assume > that your basename_r() and the implementation I did aren't too far apart > behavior-wise?) they're completely different. your basename_r is exactly the same as the basename you get from bionic/glibc's <string.h> (as opposed to the POSIX <libgen.h> one). the bionic basename_r is probably what you'd expect if you wrote a potentially-modifying basename that used the buffer it was passed. the BSD one is one you'd get if you tried that but didn't have the sense to include the size_t. > I need to dig up my AOSP build and try to chip the toolchain out to test > build toybox. (Will static bionic builds run on ubuntu's kernel?) afaik all the unit tests currently pass on Ubuntu 14.04. >> bionic, but i can't fix the past. the build failure is because the >> 32-bit bionic basename_r had the signature int basename_r(const char*, >> char*, size_t). looking ahead maybe i should add a _BIONIC_NO_CRUFT so >> non-historic 32-bit code can opt out of this kind of pollution.) > > Seriously, I'd just submit this one to the Austin group for the next > posix/susvX spec. What the gnu guys did is a way bigger deviation than > bionic here. annoying though it is that they didn't give it a different name, i think the GNU behavior's more generally useful. and, like i said, LP64 bionic doesn't have basename_r. >> commit f8b41e81fca2b53410f80c95f111c754d5eff99a >> Author: Elliott Hughes <[email protected]> >> Date: Fri Jul 10 19:11:18 2015 -0700 > > I've applied this, but I'm open to using bionic's basename_r() instead > of mine instead. FYI. > > Rob -- 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
