Gitiles
Code Review Sign In
gerrit-public.fairphone.software / fp2-dev / platform / external / toybox / 0e040df95b0b0728446fe97a6f216a01595a1e6c / . / www / cleanup.html
blob: 4b4f894f4c7b81f8f7d5c70dc8fb024303e23129 [file] [log] [blame]
Rob Landley349ff522014-01-04 13:09:42 -0600[diff] [blame]1<html><head><title>toybox cleanup</title></head>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]2<!--#include file="header.html" -->
3
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]4<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 -0500[diff] [blame]21<h1>Cleaning up the toybox code.</h1>
22
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]23<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 -0500[diff] [blame]25Lots 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 -0600[diff] [blame]34because they don't work, but because we aim for an unusual level of polish.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]35
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 -0600[diff] [blame]39Below 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 -0500[diff] [blame]42
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]43<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 -0500[diff] [blame]49directory 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 -0600[diff] [blame]54<a name=advice />
55<h1>General advice and/or policy.</h1>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]56
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]57<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 -0500[diff] [blame]62
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]63<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>
69</ul>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]70
71<hr>
72
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]73<a name="uuencode"><h1><a href=/hg/toybox/log/900/toys/pending/uuencode.c>uuencode</a></h1>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]74
75<p>This is an example of cleaning up something
76that started in a condition most projects would be quite happy with.
77The initial submission of uuencode and uudecode was a good high
78quality contribution, written by a seasoned developer who did an excellent
79job. It was still possible to shrink uuencode almost by half:</p>
80
81<ul>
82<li>old total: <a href=/hg/toybox/file/828/toys/pending/uuencode.c>116 lines (2743
83bytes) in 7 functions</a></li>
84<li>new total: <a href=/hg/toybox/file/841/toys/posix/uuencode.c>67 lines (1440
85bytes) in 1 function</a></li>
86</ul>
87
88<ul>
89<li>commit: <a href=/hg/toybox/rev/830>830</a>,
90description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000904.html>part 1</a> and
91<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000903.html>part 2</a></li>
92<li>commit: <a href=/hg/toybox/rev/831>831</a>,
93description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000919.html>inline, default Y, move to toys/posix</a></li>
94<li>commit: <a href=/hg/toybox/rev/837>837</a>,
95description: test suite.</li>
96</ul>
97
98<p>Status: COMPLETE</p>
99
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]100<a name="uudecode"><h1><a href=/hg/toybox/log/900/toys/pending/uudecode.c>uudecode</a></h1>
101
102<p>I tried to do the uudecode cleanup in smaller stages than uuencode:</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]103
104<ul>
105<li>old: <a href=/hg/toybox/file/828/toys/pending/uudecode.c>175
106lines (4534 bytes) in 9 functions</a></li>
107<li>new: <a href=/hg/toybox/file/840/toys/posix/uudecode.c>107 lines
108(2300 bytes) in 1 function</a></li>
109</ul>
110
111<ul>
112<li>commit: <a href=/hg/toybox/rev/833>833</a>,
113description: preparatory adjustments to test suite.</li>
114<li>commit: <a href=/hg/toybox/rev/835>835</a>,
115description: todo</a></li>
116<li>commit: <a href=/hg/toybox/rev/838>838</a>,
117description: todo</a></li>
118<li>commit: <a href=/hg/toybox/rev/839>839</a>,
119description: todo</a></li>
120<li>commit: <a href=/hg/toybox/rev/839>839</a>,
121description: todo (finish, move pending->posix, default y)</a></li>
122</ul>
123
124<p>Status: COMPLETE</p>
125
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]126<a name=ifconfig>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]127<h1><a href=/hg/toybox/log/tip/toys/pending/ifconfig.c>ifconfig</a></h1>
128
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]129<p>When ifconfig was submitted, it touched a half-dozen files. I glued it
130together into a single self-contained file, which needed a lot of
131cleanup. The final version is about 1/3 the size of the original.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]132
133<ul>
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]134<li>old total: <a href=/hg/toybox/file/841/toys/pending/ifconfig.c>1504 lines (44268 bytes) in 38 functions</a></li>
135<li>new total: <a href=/hg/toybox/file/1113/toys/other/ifconfig.c>521 lines (15963 bytes) in 4 function</a></li>
136</ul>
137
138<p>This was the first command I started cleaning up with the intent of
139describing the process, and especially the first few entries in this
140series describe many of the low hanging fruit techniques used to clean
141up a codebase.</p>
142
143<ul>
144<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,
145unnecessary #defines, shorter code is easier to read.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]146<li>commit: <a href=/hg/toybox/rev/844>844</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]147description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000881.html>Headers, replacing repeated code with loops,
148logical operators guaranteed to return 0 or 1, math on string constants,
149removing unnecessary variables and tests.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]150<li>commit: <a href=/hg/toybox/rev/852>852</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]151description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000921.html>hg commit numbers, documenting the obvious, ordering
152to avoid prototypes, returning void, collate local declarations,
153use error_exit(), unnecessary parentheses, inline to remove variables/functions
154used only once, using *var instead of var[0], unnecessary typecasts,
155xprintf("\n") vs xputc('\n')</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]156<li>commit: <a href=/hg/toybox/rev/856>856</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]157description: one line portability fix from Isaac Dunham</li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]158<li>commit: <a href=/hg/toybox/rev/861>861</a>
159and <a href=/hg/toybox/rev/863>863</a>,
160description:
161<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000910.html>Help
162infrastructure cleanup from Isaac Dunham</a>
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]163(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 -0500[diff] [blame]164
165<li>commit: <a href=/hg/toybox/rev/862>862</a>,
166<a href=/hg/toybox/rev/864>864</a>,
167<a href=/hg/toybox/rev/866>866</a>: todo</li>
168
169<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 -0600[diff] [blame]170description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000928.html>869:
171reorder to eliminate prototypes, put command_main() at end of file,
172870: long repeated variable prefixes, replacing struct+sscanf()+printf with a
173loop and a table (from iface_list, get_proc_info(), display_ifconfig()),
174use lib/xwrap.c functions to return void, why xstrcpy() fails closed,
175(functional comment: why multicast failed, CSLIP obsolecense), not being
176_too_ clever.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]177<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 -0600[diff] [blame]178description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000946.html>878: add xsocket(), free(NULL) is a safe NOP (posix!).
179879: inline three functions and simplify, some simplifications only show up
180after repeated inlining</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]181<li>commit: <a href=/hg/toybox/rev/883>883</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]182description: move some common code to lib/ and posix headers to toys.h.</li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]183<li>commit: <a href=/hg/toybox/rev/898>898</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]184description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000974.html>Replace ifconfig_main() if/else staircase with a loop over
185an array, genericize - prefix logic, inline a use of set_flags().</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]186<li>commit: <a href=/hg/toybox/rev/905>905</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]187description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000992.html>remove unnecessary wrapper function, inlining more functions,
188relying on the values of constants that don't change across architectures
189(binary backwards compatability), more ifconfig_main table work,
190man ioctl_list.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]191<li>commit: <a href=/hg/toybox/rev/906>906</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]192description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000994.html>More ifconfig_main() table work, remove vestigial arguments
193to functions, "a frenzy of inlining", slightly better error reporting,
194don't reinvent libc functions.</a></li>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]195<li>commit: <a href=/hg/toybox/rev/907>907</a>,
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]196<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()
197and set_hw_address(), stop validating data we supplied, remove a table
198that's overkill (two entries, we don't even loop over it), when you don't need a
199NULL terminator for array, remove unnecessary memcpy(offsetof()) with
200assignment, trusting -funsigned-char.</a></li>
201<li>commit: <a href=/hg/toybox/rev/919>919</a>,
202<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001027.html>todo whitespace damage, introduce IFREQ_OFFSZ() and poke() to
203ifconfig_main() table logic to fold more if/else parts into the table</a></li>
204<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>
205<li>commit: <a href=/hg/toybox/rev/921>921</a>, description: todo</li>
206<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>
207<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>
208<li>commit: <a href=/hg/toybox/rev/1104>1104</a>, description: todo</li>
209<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>
210<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>
211<li>commit: <a href=/hg/toybox/rev/1132>1132</a>, description: promotion from pending to other</li>
212<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 -0500[diff] [blame]213</ul>
214
Rob Landley2b55d862014-01-01 15:00:44 -0600[diff] [blame]215<p>Status: COMPLETE.</p>
Rob Landleyb7ca39c2013-09-05 23:58:35 -0500[diff] [blame]216
217<h1><a href=/hg/toybox/log/tip/toys/pending/find.c>find</a></h1>
218
219<pre>
220814 Initial version by Gurang Shastri.
221815
222816
223847 Felix Janda
224848 Whitespace (reindent from tabs -> 2 spaces)
225849 More cleanup
226867 Felix Janda Improve operator processing.
227874 Felix Janda
228875 Felix Janda
229887 Felix Janda fix -mtime
230</pre>
231
232<ul>
233<li>commit: <a href=/hg/toybox/rev/849>849</a>,
234description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000895.html>here</a></li>
235</ul>
236
237<p>Status: in progress.</p>
238
239<h1><a href=/hg/toybox/log/917/toys/pending/stat.c>stat</a></h1>
240
241<pre>
242747 initial submission
243810 whitespace
244811
245871 whitespace (reindent from 4 spaces to 2)
246872 Felix Janda cleanup
247885 Felix Janda
248 move permission formatting (777 -> -rwxrwxrwx) from ls to lib so stat can reuse it.
249886 Felix Janda remove unimplemented options and clean up help text
250910 Felix Janda Add support for stating multiple files.
251911 Felix Janda Separate stat and statfs.
252912 <a href=/hg/toybox/rev/912>commit</a> <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001019.html>description</a>
253<a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001024.html>design pondering</a>
254
255 914 916
256</pre>
257<ul>
258<li>commit: <a href=/hg/toybox/rev/917>917</a></li>
259<li>commit: <a href=/hg/toybox/rev/918>918</a>,
260description: move to posix, default y.</li>
261</ul>
262
263<p>Status: COMPLETE.</p>
264
265<!--#include file="footer.html" -->