path: root/www/cleanup.html

<!--#include file="header.html" -->

<h1>Index</h1>

<ul>
<li><a href=#intro>Introduction</a></li>
<li><a href=#advice>Advice</a></li>
<li>Commands:</li>
<ul>
<li><a href=#uuencode>uuencode</a></li>
<li><a href=#uudecode>uudecode</a></li>
<li><a href=#ifconfig>ifconfig</a></li>
<li><a href=#find>find</a></li>
</ul>
</ul>

<hr>

<a name=intro />
<h1>Cleaning up the toybox code.</h1>

<p>Toybox <a href=http://landley.net/notes.html#31-03-2013>hasn't always</a>
taken proper advantage of external contributions</a>.
Lots of people want to help, but their contributions languish out of tree
or in the "pending" directory, awaiting cleanup.</p>

<p>Toybox's design goals require simpler, tighter, and more explicit code
than most other implementations, among other reasons to allow better security
auditing. Writing "another" implementation of standard command line tools
isn't very interesting, they should be _better_ implementations.
Unfortunately, this means existing, working commands often take more effort to
clean up to Toybox's standards than writing a new one from scratch, not
because they don't work, but because we aim for an unusual level of polish.</p>

<p>In hopes of teaching more people how to do this
cleanup work, I've started breaking cleanup changes into smaller chunks and
posting explanations of each change to the mailing list.
Below are indexes of such cleanup series. Each commit and post are meant to
be read together: each description explains what the corresponding patch
was trying to accomplish.</p>

<p>Line/byte totals of completed series are given for scale, but the point
of this work is simplicity and compactness, not size per se.</p>

<p>(Note: mercurial's web viewer doesn't follow renames, so although each
command name links to a commit list with the bulk of the changes, it may
not include the final version of each file moved from the "pending"
directory to its final location. The summaries link the initial and cleaned
versions of each file when giving line counts.)</p>

<hr>

<a name=advice />
<h1>General advice and/or policy.</h1>

<p>The <a href=design.html>design of toybox</a> page and the coding
style section at the start of the <a href=code.html>source code walkthrough</a>
don't cover everything. Here are some
links to mailing list messages that cover various programming topics
not directly related to a specific cleanup series:</p>

<ul>
<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html>Error messages and internationalization.</a></li>
<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000891.html>Why not "const"?</a></li>
<li><a href=http://lkml.indiana.edu/hypermail/linux/kernel/1308.3/03890.html>Why not "bool"?</a> (explanation from Linus Torvalds)</li>
<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000893.html>Why not to check in debug code.</a></li>
<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001044.html>Relationship between /proc and /sys</a> (/proc isn't obsolete and /sys is an ABI)</li>
</ul>

<hr>

<a name="uuencode"><h1><a href=/hg/toybox/log/900/toys/pending/uuencode.c>uuencode</a></h1>

<p>This is an example of cleaning up something
that started in a condition most projects would be quite happy with.
The initial submission of uuencode and uudecode was a good high
quality contribution, written by a seasoned developer who did an excellent
job. It was still possible to shrink uuencode almost by half:</p>

<ul>
<li>old total: <a href=/hg/toybox/file/828/toys/pending/uuencode.c>116 lines (2743
bytes) in 7 functions</a></li>
<li>new total: <a href=/hg/toybox/file/841/toys/posix/uuencode.c>67 lines (1440
bytes) in 1 function</a></li>
</ul>

<ul>
<li>commit: <a href=/hg/toybox/rev/830>830</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000904.html>part 1</a> and
<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000903.html>part 2</a></li>
<li>commit: <a href=/hg/toybox/rev/831>831</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000919.html>inline, default Y, move to toys/posix</a></li>
<li>commit: <a href=/hg/toybox/rev/837>837</a>,
description: test suite.</li>
</ul>

<p>Status: COMPLETE</p>

<a name="uudecode"><h1><a href=/hg/toybox/log/900/toys/pending/uudecode.c>uudecode</a></h1>

<p>I tried to do the uudecode cleanup in smaller stages than uuencode:</p>

<ul>
<li>old: <a href=/hg/toybox/file/828/toys/pending/uudecode.c>175
lines (4534 bytes) in 9 functions</a></li>
<li>new: <a href=/hg/toybox/file/840/toys/posix/uudecode.c>107 lines
(2300 bytes) in 1 function</a></li>
</ul>

<ul>
<li>commit: <a href=/hg/toybox/rev/833>833</a>,
description: preparatory adjustments to test suite.</li>
<li>commit: <a href=/hg/toybox/rev/835>835</a>,
description: todo</a></li>
<li>commit: <a href=/hg/toybox/rev/838>838</a>,
description: todo</a></li>
<li>commit: <a href=/hg/toybox/rev/839>839</a>,
description: todo</a></li>
<li>commit: <a href=/hg/toybox/rev/839>839</a>,
description: todo (finish, move pending->posix, default y)</a></li>
</ul>

<p>Status: COMPLETE</p>

<a name=ifconfig>
<h1><a href=/hg/toybox/log/tip/toys/pending/ifconfig.c>ifconfig</a></h1>

<p>When ifconfig was submitted, it touched a half-dozen files. I glued it
together into a single self-contained file, which needed a lot of
cleanup. The final version is about 1/3 the size of the original.</p>

<ul>
<li>old total: <a href=/hg/toybox/file/841/toys/pending/ifconfig.c>1504 lines (44268 bytes) in 38 functions</a></li>
<li>new total: <a href=/hg/toybox/file/1113/toys/other/ifconfig.c>521 lines (15963 bytes) in 4 function</a></li>
</ul>

<p>This was the first command I started cleaning up with the intent of
describing the process, and especially the first few entries in this
series describe many of the low hanging fruit techniques used to clean
up a codebase.</p>

<ul>
<li>commit: <a href=/hg/toybox/rev/843>843</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000882.html>Infrastructure in search of a user, code proximity,
unnecessary #defines, shorter code is easier to read.</a></li>
<li>commit: <a href=/hg/toybox/rev/844>844</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000881.html>Headers, replacing repeated code with loops,
logical operators guaranteed to return 0 or 1, math on string constants,
removing unnecessary variables and tests.</a></li>
<li>commit: <a href=/hg/toybox/rev/852>852</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000921.html>hg commit numbers, documenting the obvious, ordering
to avoid prototypes, returning void, collate local declarations,
use error_exit(), unnecessary parentheses, inline to remove variables/functions
used only once, using *var instead of var[0], unnecessary typecasts,
xprintf("\n") vs xputc('\n')</a></li>
<li>commit: <a href=/hg/toybox/rev/856>856</a>,
description: one line portability fix from Isaac Dunham</li>
<li>commit: <a href=/hg/toybox/rev/861>861</a>
and <a href=/hg/toybox/rev/863>863</a>,
description:
<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000910.html>Help
infrastructure cleanup from Isaac Dunham</a>
(which I mis-applied and then <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000920.html>fixed plus some whitespace changes</a>)</li>

<li>commit: <a href=/hg/toybox/rev/862>862</a>,
<a href=/hg/toybox/rev/864>864</a>,
<a href=/hg/toybox/rev/866>866</a>: todo</li>

<li>commit: <a href=/hg/toybox/rev/869>869</a> and <a href=/hg/toybox/rev/870>870</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000928.html>869:
reorder to eliminate prototypes, put command_main() at end of file,
870: long repeated variable prefixes, replacing struct+sscanf()+printf with a
loop and a table (from iface_list, get_proc_info(), display_ifconfig()),
use lib/xwrap.c functions to return void, why xstrcpy() fails closed,
(functional comment: why multicast failed, CSLIP obsolecense), not being
_too_ clever.</a></li>
<li>commit: <a href=/hg/toybox/rev/878>878</a> and <a href=/hg/toybox/rev/879>879</a>:
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000946.html>878: add xsocket(), free(NULL) is a safe NOP (posix!).
879: inline three functions and simplify, some simplifications only show up
after repeated inlining</a></li>
<li>commit: <a href=/hg/toybox/rev/883>883</a>,
description: move some common code to lib/ and posix headers to toys.h.</li>
<li>commit: <a href=/hg/toybox/rev/898>898</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000974.html>Replace ifconfig_main() if/else staircase with a loop over
an array, genericize - prefix logic, inline a use of set_flags().</a></li>
<li>commit: <a href=/hg/toybox/rev/905>905</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000992.html>remove unnecessary wrapper function, inlining more functions,
relying on the values of constants that don't change across architectures
(binary backwards compatability), more ifconfig_main table work,
man ioctl_list.</a></li>
<li>commit: <a href=/hg/toybox/rev/906>906</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000994.html>More ifconfig_main() table work, remove vestigial arguments
to functions, "a frenzy of inlining", slightly better error reporting,
don't reinvent libc functions.</a></li>
<li>commit: <a href=/hg/toybox/rev/907>907</a>,
<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000995.html>inlining show_ip_addr() with a loop and a table, inlining hex_to_binary()
and set_hw_address(), stop validating data we supplied, remove a table
that's overkill (two entries, we don't even loop over it), when you don't need a
NULL terminator for array, remove unnecessary memcpy(offsetof()) with
assignment, trusting -funsigned-char.</a></li>
<li>commit: <a href=/hg/toybox/rev/919>919</a>,
<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001027.html>todo whitespace damage, introduce IFREQ_OFFSZ() and poke() to
ifconfig_main() table logic to fold more if/else parts into the table</a></li>
<li>Status update: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001033.html>Entering the home stretch on ifconfig</a> (and a <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001043.html>note about infiniband</a>)</li>
<li>commit: <a href=/hg/toybox/rev/921>921</a>, description: todo</li>
<li>commit: <a href=/hg/toybox/rev/957>957</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-July/001121.html>here</a></li>
<li>commit: <a href=/hg/toybox/rev/958>958</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-July/001131.html>here</a></li>
<li>commit: <a href=/hg/toybox/rev/1104>1104</a>, description: todo</li>
<li>commit: <a href=/hg/toybox/rev/1127>1127</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-November/001464.html>here</a></li>
<li>commit: <a href=/hg/toybox/rev/1128>1128</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-November/001463.html>here</a></li>
<li>commit: <a href=/hg/toybox/rev/1132>1132</a>, description: promotion from pending to other</li>
<li>commit: <a href=/hg/toybox/rev/1132>1133</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-November/001462.html>cleanup help text, remove obsolete/NOP commands</a></li>
</ul>

<p>Status: COMPLETE.</p>

<h1><a href=/hg/toybox/log/tip/toys/pending/find.c>find</a></h1>

<pre>
814 Initial version by Gurang Shastri.
815
816
847 Felix Janda
848 Whitespace (reindent from tabs -> 2 spaces)
849 More cleanup
867 Felix Janda Improve operator processing.
874 Felix Janda
875 Felix Janda
887 Felix Janda fix -mtime
</pre>

<ul>
<li>commit: <a href=/hg/toybox/rev/849>849</a>,
description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000895.html>here</a></li>
</ul>

<p>Status: in progress.</p>

<h1><a href=/hg/toybox/log/917/toys/pending/stat.c>stat</a></h1>

<pre>
747 initial submission
810 whitespace
811
871 whitespace (reindent from 4 spaces to 2)
872 Felix Janda cleanup
885 Felix Janda
  move permission formatting (777 -> -rwxrwxrwx) from ls to lib so stat can reuse it.
886 Felix Janda remove unimplemented options and clean up help text
910 Felix Janda Add support for stating multiple files.
911 Felix Janda Separate stat and statfs.
912 <a href=/hg/toybox/rev/912>commit</a> <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001019.html>description</a>
<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001024.html>design pondering</a>

 914 916
</pre>
<ul>
<li>commit: <a href=/hg/toybox/rev/917>917</a></li>
<li>commit: <a href=/hg/toybox/rev/918>918</a>,
description: move to posix, default y.</li>
</ul>

<p>Status: COMPLETE.</p>

<!--#include file="footer.html" -->