blob: 59199947769e7093300be2d7f2881a33baaf7a48 [file] [log] [blame]
Rob Landley349ff522014-01-04 13:09:42 -06001<html><head><title>toybox cleanup</title></head>
Rob Landleyb7ca39c2013-09-05 23:58:35 -05002<!--#include file="header.html" -->
3
Rob Landley2b55d862014-01-01 15:00:44 -06004<h1>Index</h1>
5
6<ul>
7<li><a href=#intro>Introduction</a></li>
8<li><a href=#advice>Advice</a></li>
9<li>Commands:</li>
10<ul>
11<li><a href=#uuencode>uuencode</a></li>
12<li><a href=#uudecode>uudecode</a></li>
13<li><a href=#ifconfig>ifconfig</a></li>
14<li><a href=#find>find</a></li>
15</ul>
16</ul>
17
18<hr>
19
20<a name=intro />
Rob Landleyb7ca39c2013-09-05 23:58:35 -050021<h1>Cleaning up the toybox code.</h1>
22
Rob Landley2b55d862014-01-01 15:00:44 -060023<p>Toybox <a href=http://landley.net/notes.html#31-03-2013>hasn't always</a>
24taken proper advantage of external contributions</a>.
Rob Landleyb7ca39c2013-09-05 23:58:35 -050025Lots of people want to help, but their contributions languish out of tree
26or in the "pending" directory, awaiting cleanup.</p>
27
28<p>Toybox's design goals require simpler, tighter, and more explicit code
29than most other implementations, among other reasons to allow better security
30auditing. Writing "another" implementation of standard command line tools
31isn't very interesting, they should be _better_ implementations.
32Unfortunately, this means existing, working commands often take more effort to
33clean up to Toybox's standards than writing a new one from scratch, not
Rob Landley2b55d862014-01-01 15:00:44 -060034because they don't work, but because we aim for an unusual level of polish.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050035
36<p>In hopes of teaching more people how to do this
37cleanup work, I've started breaking cleanup changes into smaller chunks and
38posting explanations of each change to the mailing list.
Rob Landley2b55d862014-01-01 15:00:44 -060039Below are indexes of such cleanup series. Each commit and post are meant to
40be read together: each description explains what the corresponding patch
41was trying to accomplish.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050042
Rob Landley2b55d862014-01-01 15:00:44 -060043<p>Line/byte totals of completed series are given for scale, but the point
44of this work is simplicity and compactness, not size per se.</p>
45
46<p>(Note: mercurial's web viewer doesn't follow renames, so although each
47command name links to a commit list with the bulk of the changes, it may
48not include the final version of each file moved from the "pending"
Rob Landleyb7ca39c2013-09-05 23:58:35 -050049directory to its final location. The summaries link the initial and cleaned
50versions of each file when giving line counts.)</p>
51
52<hr>
53
Rob Landley2b55d862014-01-01 15:00:44 -060054<a name=advice />
55<h1>General advice and/or policy.</h1>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050056
Rob Landley2b55d862014-01-01 15:00:44 -060057<p>The <a href=design.html>design of toybox</a> page and the coding
58style section at the start of the <a href=code.html>source code walkthrough</a>
59don't cover everything. Here are some
60links to mailing list messages that cover various programming topics
61not directly related to a specific cleanup series:</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050062
Rob Landley2b55d862014-01-01 15:00:44 -060063<ul>
64<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html>Error messages and internationalization.</a></li>
65<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000891.html>Why not "const"?</a></li>
66<li><a href=http://lkml.indiana.edu/hypermail/linux/kernel/1308.3/03890.html>Why not "bool"?</a> (explanation from Linus Torvalds)</li>
67<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000893.html>Why not to check in debug code.</a></li>
68<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>
Rob Landleydc62a012014-07-26 13:22:59 -050069<li>"Hiding numbers that are used just once into defines to put them out of
70sight does not really help readability."</a> -
71<a href=http://lkml.iu.edu/hypermail/linux/kernel/1407.1/00299.html>Pavel
72Machek</a></li>
Rob Landley2b55d862014-01-01 15:00:44 -060073</ul>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050074
75<hr>
76
Rob Landley2b55d862014-01-01 15:00:44 -060077<a name="uuencode"><h1><a href=/hg/toybox/log/900/toys/pending/uuencode.c>uuencode</a></h1>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050078
Rob Landleya89f8aa2014-02-04 06:16:44 -060079<p>This is an example of cleaning up something most projects would be quite
80happy with. The initial submission of uuencode and uudecode was high
81quality code, written by a seasoned developer who did an excellent
82job, but it was still possible to shrink the result almost by half:</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050083
84<ul>
85<li>old total: <a href=/hg/toybox/file/828/toys/pending/uuencode.c>116 lines (2743
86bytes) in 7 functions</a></li>
87<li>new total: <a href=/hg/toybox/file/841/toys/posix/uuencode.c>67 lines (1440
88bytes) in 1 function</a></li>
89</ul>
90
91<ul>
Rob Landleya89f8aa2014-02-04 06:16:44 -060092<li>commit: <a href=/hg/toybox/rev/830>830</a>: first pass, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000904.html>part 1</a>,
Rob Landleyb7ca39c2013-09-05 23:58:35 -050093<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000903.html>part 2</a></li>
94<li>commit: <a href=/hg/toybox/rev/831>831</a>,
Rob Landleya89f8aa2014-02-04 06:16:44 -060095second pass, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000919.html>part 3</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050096<li>commit: <a href=/hg/toybox/rev/837>837</a>,
Rob Landleya89f8aa2014-02-04 06:16:44 -060097description: fix test suite.</li>
98<li>commit: <a href=/hg/toybox/rev/853>853</a>, description: bugfix.</li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -050099</ul>
100
101<p>Status: COMPLETE</p>
102
Rob Landley2b55d862014-01-01 15:00:44 -0600103<a name="uudecode"><h1><a href=/hg/toybox/log/900/toys/pending/uudecode.c>uudecode</a></h1>
104
Rob Landleya89f8aa2014-02-04 06:16:44 -0600105<p>The uudecode cleanup was my second "explain as I go along" cleanup,
106and I tried to do it in smaller stages so it was easier to see what
107changed from the diff:</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500108
109<ul>
110<li>old: <a href=/hg/toybox/file/828/toys/pending/uudecode.c>175
111lines (4534 bytes) in 9 functions</a></li>
112<li>new: <a href=/hg/toybox/file/840/toys/posix/uudecode.c>107 lines
113(2300 bytes) in 1 function</a></li>
114</ul>
115
116<ul>
117<li>commit: <a href=/hg/toybox/rev/833>833</a>,
118description: preparatory adjustments to test suite.</li>
119<li>commit: <a href=/hg/toybox/rev/835>835</a>,
Rob Landleydc62a012014-07-26 13:22:59 -0500120description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2014-January/001532.html>Redo command line parsing, redo parsing loop.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500121<li>commit: <a href=/hg/toybox/rev/838>838</a>,
Rob Landleydc62a012014-07-26 13:22:59 -0500122description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2014-January/001533.html>Redo b64_1byte, b64_4bytes, and uu_line()</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500123<li>commit: <a href=/hg/toybox/rev/839>839</a>,
124description: todo</a></li>
Rob Landleya89f8aa2014-02-04 06:16:44 -0600125<li>commit: <a href=/hg/toybox/rev/840>840</a>,
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500126description: todo (finish, move pending->posix, default y)</a></li>
127</ul>
128
129<p>Status: COMPLETE</p>
130
Rob Landley2b55d862014-01-01 15:00:44 -0600131<a name=ifconfig>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500132<h1><a href=/hg/toybox/log/tip/toys/pending/ifconfig.c>ifconfig</a></h1>
133
Rob Landleya89f8aa2014-02-04 06:16:44 -0600134<p>This series describes a thorough cleanup that took a while to do.</p>
135
Rob Landley2b55d862014-01-01 15:00:44 -0600136<p>When ifconfig was submitted, it touched a half-dozen files. I glued it
137together into a single self-contained file, which needed a lot of
138cleanup. The final version is about 1/3 the size of the original.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500139
140<ul>
Rob Landley2b55d862014-01-01 15:00:44 -0600141<li>old total: <a href=/hg/toybox/file/841/toys/pending/ifconfig.c>1504 lines (44268 bytes) in 38 functions</a></li>
Rob Landleya89f8aa2014-02-04 06:16:44 -0600142<li>new total: <a href=/hg/toybox/file/1113/toys/other/ifconfig.c>521 lines (15963 bytes) in 4 functions</a></li>
Rob Landley2b55d862014-01-01 15:00:44 -0600143</ul>
144
145<p>This was the first command I started cleaning up with the intent of
146describing the process, and especially the first few entries in this
147series describe many of the low hanging fruit techniques used to clean
148up a codebase.</p>
149
150<ul>
151<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,
152unnecessary #defines, shorter code is easier to read.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500153<li>commit: <a href=/hg/toybox/rev/844>844</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600154description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000881.html>Headers, replacing repeated code with loops,
155logical operators guaranteed to return 0 or 1, math on string constants,
156removing unnecessary variables and tests.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500157<li>commit: <a href=/hg/toybox/rev/852>852</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600158description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000921.html>hg commit numbers, documenting the obvious, ordering
159to avoid prototypes, returning void, collate local declarations,
160use error_exit(), unnecessary parentheses, inline to remove variables/functions
161used only once, using *var instead of var[0], unnecessary typecasts,
162xprintf("\n") vs xputc('\n')</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500163<li>commit: <a href=/hg/toybox/rev/856>856</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600164description: one line portability fix from Isaac Dunham</li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500165<li>commit: <a href=/hg/toybox/rev/861>861</a>
166and <a href=/hg/toybox/rev/863>863</a>,
167description:
168<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000910.html>Help
169infrastructure cleanup from Isaac Dunham</a>
Rob Landley2b55d862014-01-01 15:00:44 -0600170(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>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500171
Rob Landleya89f8aa2014-02-04 06:16:44 -0600172<li>commit: <a href=/hg/toybox/rev/862>862</a>, description:
173<a href=http://lists.landley.net/pipermail/toybox-landley.net/2014-January/001525.html>remove unused headers and function, replace local buffer with toybuf, perror_exit(), avoid unnecessary assignment.</a></li>
174<li>commit: <a href=/hg/toybox/rev/864>864</a>, description:
Rob Landleyf23d3172014-06-27 22:26:02 -0500175<a href=http://lists.landley.net/pipermail/toybox-landley.net/2014-January/001526.html>use common linked list functions, inline set_data, add xioctl, clean up error messages, whitespace and comment tweaks, remove NOP return statements</a></li>
Rob Landleya89f8aa2014-02-04 06:16:44 -0600176<li>commit: <a href=/hg/toybox/rev/866>866</a>, description:
Rob Landleyf23d3172014-06-27 22:26:02 -0500177<a href=http://lists.landley.net/pipermail/toybox-landley.net/2014-January/001528.html>move standalone globals into GLOBALS() block, collate structs into
178iface_list. Inline/rewrite/remove field_format, iface_flags_str,
179omit_whitespace(), print_iface_flags(), print_media(), get_ifconfig_info(),
180and clear_list(). Merge duplicate function
181calls. Show get_proc_info() version field can't matter in 2.6 or newer kernel,
182and that SIOCGIFMAP has been there since 1994 so doesn't need an #ifdef.
183Loop simplification in readconf() and show_iface().</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500184
185<li>commit: <a href=/hg/toybox/rev/869>869</a> and <a href=/hg/toybox/rev/870>870</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600186description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000928.html>869:
187reorder to eliminate prototypes, put command_main() at end of file,
188870: long repeated variable prefixes, replacing struct+sscanf()+printf with a
189loop and a table (from iface_list, get_proc_info(), display_ifconfig()),
190use lib/xwrap.c functions to return void, why xstrcpy() fails closed,
191(functional comment: why multicast failed, CSLIP obsolecense), not being
192_too_ clever.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500193<li>commit: <a href=/hg/toybox/rev/878>878</a> and <a href=/hg/toybox/rev/879>879</a>:
Rob Landley2b55d862014-01-01 15:00:44 -0600194description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000946.html>878: add xsocket(), free(NULL) is a safe NOP (posix!).
195879: inline three functions and simplify, some simplifications only show up
196after repeated inlining</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500197<li>commit: <a href=/hg/toybox/rev/883>883</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600198description: move some common code to lib/ and posix headers to toys.h.</li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500199<li>commit: <a href=/hg/toybox/rev/898>898</a>,
Rob Landleya89f8aa2014-02-04 06:16:44 -0600200description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000974.html>Argument parsing. (Replace ifconfig_main() if/else staircase with a loop over
201an array, genericize minus prefix logic, inline a use of set_flags().)</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500202<li>commit: <a href=/hg/toybox/rev/905>905</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600203description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000992.html>remove unnecessary wrapper function, inlining more functions,
204relying on the values of constants that don't change across architectures
205(binary backwards compatability), more ifconfig_main table work,
206man ioctl_list.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500207<li>commit: <a href=/hg/toybox/rev/906>906</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600208description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000994.html>More ifconfig_main() table work, remove vestigial arguments
209to functions, "a frenzy of inlining", slightly better error reporting,
210don't reinvent libc functions.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500211<li>commit: <a href=/hg/toybox/rev/907>907</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600212<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()
213and set_hw_address(), stop validating data we supplied, remove a table
214that's overkill (two entries, we don't even loop over it), when you don't need a
215NULL terminator for array, remove unnecessary memcpy(offsetof()) with
216assignment, trusting -funsigned-char.</a></li>
217<li>commit: <a href=/hg/toybox/rev/919>919</a>,
218<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001027.html>todo whitespace damage, introduce IFREQ_OFFSZ() and poke() to
219ifconfig_main() table logic to fold more if/else parts into the table</a></li>
220<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>
Rob Landleyf23d3172014-06-27 22:26:02 -0500221<li>commit: <a href=/hg/toybox/rev/921>921</a>, description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2014-June/003508.html>Inline a couple more functions and make sockfd a global.</li>
222<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>Remove unused socklen and addr_to_len(), cleanup so we can merge get_device_info()/display_ifconfig().</a></li>
223<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>This commit removes struct if_list by unifying get_device_info() and display_ifconfig().</a></li>
224<li>commit: <a href=/hg/toybox/rev/1104>1104</a>, description: Merge toynet into toys.h: musl supports it and micromanaging uClibc config options isn't very interesting anymore.</li>
225<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>Start tacling ipv6 issues, beginning with display_ifconfig().</a></li>
226<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>More ipv6, make struct sockaddr_with_len go away, merge more arguments into the table in main().</a></li>
Rob Landley2b55d862014-01-01 15:00:44 -0600227<li>commit: <a href=/hg/toybox/rev/1132>1132</a>, description: promotion from pending to other</li>
228<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>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500229</ul>
230
Rob Landley2b55d862014-01-01 15:00:44 -0600231<p>Status: COMPLETE.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500232
233<h1><a href=/hg/toybox/log/tip/toys/pending/find.c>find</a></h1>
234
235<pre>
236814 Initial version by Gurang Shastri.
237815
238816
239847 Felix Janda
240848 Whitespace (reindent from tabs -> 2 spaces)
241849 More cleanup
242867 Felix Janda Improve operator processing.
243874 Felix Janda
244875 Felix Janda
245887 Felix Janda fix -mtime
246</pre>
247
248<ul>
249<li>commit: <a href=/hg/toybox/rev/849>849</a>,
250description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000895.html>here</a></li>
251</ul>
252
253<p>Status: in progress.</p>
254
255<h1><a href=/hg/toybox/log/917/toys/pending/stat.c>stat</a></h1>
256
Rob Landleya89f8aa2014-02-04 06:16:44 -0600257<p>A lot of the stat cleanup was done by Felix Janda.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500258
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500259<ul>
Rob Landleya89f8aa2014-02-04 06:16:44 -0600260<li>commit <a href=/hg/toybox/rev/747>747</a>: initial submission</a></li>
261<li>commit <a href=/hg/toybox/rev/810>810</a>: whitespace</li>
262<li>commit <a href=/hg/toybox/rev/811>811</a>: description in commit message.</li>
263<li>commit <a href=/hg/toybox/rev/871>871</a>: whitespace (reindent from 4 spaces to 2)</li>
264<li>commit <a href=/hg/toybox/rev/872>872</a>: Felix Janda - <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000923.html>cleanup</a> (with discussion thread)</li>
265<li>commit <a href=/hg/toybox/rev/875>885</a>: Felix Janda - <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000936.html>move permission formatting (777 -> -rwxrwxrwx) from ls to lib so stat can reuse it.</a></li>
266<li>commit <a href=/hg/toybox/rev/885>886</a>: Felix Janda - remove unimplemented options and clean up help text</li>
267<li>commit <a href=/hg/toybox/rev/910>910</a>: Felix Janda - <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001013.html>Add support for stating multiple files</a>.</li>
268<li>commit <a href=/hg/toybox/rev/911>911</a>: Felix Janda - Separate stat and statfs, give stat_main() a ds[2] array to distinguish FLAG_f vs not cases, and some whitespace changes.</li>
269<li>commit <a href=/hg/toybox/rev/912>912</a>: description in commit message (also <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001019.html>here</a>)</li>
270<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001024.html>design pondering</a> (leading to peek() function in lib/)</li>
271
272<li>commit <a href=/hg/toybox/rev/914>914</a>: description in commit message.</li>
273<li>commit <a href=/hg/toybox/rev/916>916</a>: description in commit message.</li>
274<li>commit: <a href=/hg/toybox/rev/917>917</a>: description in commit message.</li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500275<li>commit: <a href=/hg/toybox/rev/918>918</a>,
Rob Landleya89f8aa2014-02-04 06:16:44 -0600276description: done: move pending to posix, default y, no other changes</a>.</li>
277<li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001026.html>summary</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500278</ul>
279
280<p>Status: COMPLETE.</p>
281
282<!--#include file="footer.html" -->