[Toybox] sort.c patch, was: Re: PM: code style, was: Re: New Subscriber

[Rob Landley]

On 02/09/2012 03:37 AM, Frank Bergmann wrote:
> Hi,
> 
> On Wed, Feb 08, 2012 at 07:34:10PM -0600, Rob Landley wrote:
>> I fixed the sort bug yesterday morning:
>>
>>   http://landley.net/hg/toybox/rev/433
>>
>> It conflicts with your patch. If there's still an aliasing warning for
>> you, can you send me a patch on top of the current repository?
> 
> maybe my hg is broken but I don't think it is. I did a update and then
> applied the patch to the current rev.

I missed the fact you'd quoted _my_ previous patch, which was the
conflict. :)

My bad.

> TT.lines is char** and used as char*, char is 8 bit / void "no bit" and
> void* must be interchangeable with xxxxxx* - IMHO this must be valid c99 but
> I didn't test it.

void * automatically get promoted in C.  (In C++ they must be explicitly
typecast, but luckily we're not doing C++.)

> BTW - the last value of TT.lines is free()ed before assigning line as new
> value.

Yes: leaking memory on potentially infinite input is bad.

> I guess you use malloc of the C-lib, aren't you?

We're writing in C, it comes with the territory.

> For tools with
> low running times and small memory usage it could be worth using an
> allocation function as wrapper like alloc() by DJB.

Here are toolchains that link against uClibc:

  http://landley.net/aboriginal/bin

Last I checked it had something like 3 different allocators you could
pick in menuconfig when you built it.

I'm also working on putting together a toolchain that links against
bionic, and there's the musl guys, and...

A) This is "build environment", I.E. "not my problem".  My job there is
to stay out of your way.  If you want to swap out malloc() for a
different malloc(), be my guest.  You can do so as part of your build
environment.  ($CFLAGS should work.)

B) I personally find DJB annoying, and try to ignore him rather than
ranting about him.

> Instructing your authors to use this is far more easy than using a string
> class: vim/sed does the job. ;-)

Should I instruct people on how to use valgrind, strace, and operf as
well?  (What's wrong with malloc?)

> My local copy:
> http://www.tuxad.com/ngtx/ngtx-current/lib/alloc.c
> IMHO you know it already because DJB's daemontools package was mentioned
> on the bb mailing list (Denis Vlasenko and runit).

You're assuming I've been paying waaaay more attention to the busybox
list than I actually have since I left.  But I was vaguely aware of
Denys adding his pet toolset to busybox, which I felt those had no
business being in busybox since we already had an init and they weren't
present in any system I was familiar with, but he was maintainer now and
I wasn't going to criticize his decisions...

(And I followed the "what license is qmail under", "go away and stop
bothering me" decade of people using software the guy refused to
clarify.  And which nobody else could patch because the had no terms...)

(And despite what ESR said it's not "bernstein chaining" if the same
darn technique was already used by nice, detach, strace, sudo, and a
gazillion other things...)

Sigh.  Sore spot with me...

Rob