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

[Frank Bergmann]

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.

Doing update again it does nothing:
$ hg update
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg log toys/sort.c 
changeset:   433:9e7aaecf0683
tag:         tip
user:        Rob Landley <rob at landley.net>
date:        Tue Feb 07 00:31:37 2012 -0600
summary:     Fix sort -uc (pointer vs pointer to pointer confusion, covered by typecast).

(I'm new to mercurial. I did also search a way of recovering the
timestamps. It took me some time until I got a line doing the job. rcs,
cvs and svn I used many years (seldom git) but I'm a newbie regarding
mercurial.)

I still got the same diff:
$ diff -u ../toybox-cloned-1328632507/toys/sort.c
toys/sort.c
--- ../toybox-cloned-1328632507/toys/sort.c     2012-02-07
17:35:07.000000000 +0100
+++ toys/sort.c 2012-02-07 17:37:41.000000000 +0100
@@ -308,7 +308,7 @@
         if (CFG_SORT_BIG && (toys.optflags&FLAG_c)) {
             int j = (toys.optflags&FLAG_u) ? -1 : 0;

-            if (TT.lines && compare_keys((char **)&TT.lines, &line)>j)
+            if (TT.lines && compare_keys((void *)&TT.lines, &line)>j)   

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.

BTW - the last value of TT.lines is free()ed before assigning line as new
value. I guess you use malloc of the C-lib, aren't you? For tools with
low running times and small memory usage it could be worth using an
allocation function as wrapper like alloc() by DJB.
Instructing your authors to use this is far more easy than using a string
class: vim/sed does the job. ;-)
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).

Frank

-- 
EDV Frank Bergmann                           Tel.     05221-9249753
LPIC-3 Linux Professional                    Fax      05221-9249754
Pödinghauser Str. 5                          email    iservice at tuxad.com
32051 Herford                                USt-IdNr DE237314606