www/cleanup.html

   1 <!--#include file="header.html" -->
   2
   3 <h1>Index</h1>
   4
   5 <ul>
   6 <li><a href=#intro>Introduction</a></li>
   7 <li><a href=#advice>Advice</a></li>
   8 <li>Commands:</li>
   9 <ul>
  10 <li><a href=#uuencode>uuencode</a></li>
  11 <li><a href=#uudecode>uudecode</a></li>
  12 <li><a href=#ifconfig>ifconfig</a></li>
  13 <li><a href=#find>find</a></li>
  14 </ul>
  15 </ul>
  16
  17 <hr>
  18
  19 <a name=intro />
  20 <h1>Cleaning up the toybox code.</h1>
  21
  22 <p>Toybox <a href=http://landley.net/notes.html#31-03-2013>hasn't always</a>
  23 taken proper advantage of external contributions</a>.
  24 Lots of people want to help, but their contributions languish out of tree
  25 or in the "pending" directory, awaiting cleanup.</p>
  26
  27 <p>Toybox's design goals require simpler, tighter, and more explicit code
  28 than most other implementations, among other reasons to allow better security
  29 auditing. Writing "another" implementation of standard command line tools
  30 isn't very interesting, they should be _better_ implementations.
  31 Unfortunately, this means existing, working commands often take more effort to
  32 clean up to Toybox's standards than writing a new one from scratch, not
  33 because they don't work, but because we aim for an unusual level of polish.</p>
  34
  35 <p>In hopes of teaching more people how to do this
  36 cleanup work, I've started breaking cleanup changes into smaller chunks and
  37 posting explanations of each change to the mailing list.
  38 Below are indexes of such cleanup series. Each commit and post are meant to
  39 be read together: each description explains what the corresponding patch
  40 was trying to accomplish.</p>
  41
  42 <p>Line/byte totals of completed series are given for scale, but the point
  43 of this work is simplicity and compactness, not size per se.</p>
  44
  45 <p>(Note: mercurial's web viewer doesn't follow renames, so although each
  46 command name links to a commit list with the bulk of the changes, it may
  47 not include the final version of each file moved from the "pending"
  48 directory to its final location. The summaries link the initial and cleaned
  49 versions of each file when giving line counts.)</p>
  50
  51 <hr>
  52
  53 <a name=advice />
  54 <h1>General advice and/or policy.</h1>
  55
  56 <p>The <a href=design.html>design of toybox</a> page and the coding
  57 style section at the start of the <a href=code.html>source code walkthrough</a>
  58 don't cover everything. Here are some
  59 links to mailing list messages that cover various programming topics
  60 not directly related to a specific cleanup series:</p>
  61
  62 <ul>
  63 <li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html>Error messages and internationalization.</a></li>
  64 <li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000891.html>Why not "const"?</a></li>
  65 <li><a href=http://lkml.indiana.edu/hypermail/linux/kernel/1308.3/03890.html>Why not "bool"?</a> (explanation from Linus Torvalds)</li>
  66 <li><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000893.html>Why not to check in debug code.</a></li>
  67 <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>
  68 </ul>
  69
  70 <hr>
  71
  72 <a name="uuencode"><h1><a href=/hg/toybox/log/900/toys/pending/uuencode.c>uuencode</a></h1>
  73
  74 <p>This is an example of cleaning up something
  75 that started in a condition most projects would be quite happy with.
  76 The initial submission of uuencode and uudecode was a good high
  77 quality contribution, written by a seasoned developer who did an excellent
  78 job. It was still possible to shrink uuencode almost by half:</p>
  79
  80 <ul>
  81 <li>old total: <a href=/hg/toybox/file/828/toys/pending/uuencode.c>116 lines (2743
  82 bytes) in 7 functions</a></li>
  83 <li>new total: <a href=/hg/toybox/file/841/toys/posix/uuencode.c>67 lines (1440
  84 bytes) in 1 function</a></li>
  85 </ul>
  86
  87 <ul>
  88 <li>commit: <a href=/hg/toybox/rev/830>830</a>,
  89 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000904.html>part 1</a> and
  90 <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000903.html>part 2</a></li>
  91 <li>commit: <a href=/hg/toybox/rev/831>831</a>,
  92 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000919.html>inline, default Y, move to toys/posix</a></li>
  93 <li>commit: <a href=/hg/toybox/rev/837>837</a>,
  94 description: test suite.</li>
  95 </ul>
  96
  97 <p>Status: COMPLETE</p>
  98
  99 <a name="uudecode"><h1><a href=/hg/toybox/log/900/toys/pending/uudecode.c>uudecode</a></h1>
 100
 101 <p>I tried to do the uudecode cleanup in smaller stages than uuencode:</p>
 102
 103 <ul>
 104 <li>old: <a href=/hg/toybox/file/828/toys/pending/uudecode.c>175
 105 lines (4534 bytes) in 9 functions</a></li>
 106 <li>new: <a href=/hg/toybox/file/840/toys/posix/uudecode.c>107 lines
 107 (2300 bytes) in 1 function</a></li>
 108 </ul>
 109
 110 <ul>
 111 <li>commit: <a href=/hg/toybox/rev/833>833</a>,
 112 description: preparatory adjustments to test suite.</li>
 113 <li>commit: <a href=/hg/toybox/rev/835>835</a>,
 114 description: todo</a></li>
 115 <li>commit: <a href=/hg/toybox/rev/838>838</a>,
 116 description: todo</a></li>
 117 <li>commit: <a href=/hg/toybox/rev/839>839</a>,
 118 description: todo</a></li>
 119 <li>commit: <a href=/hg/toybox/rev/839>839</a>,
 120 description: todo (finish, move pending->posix, default y)</a></li>
 121 </ul>
 122
 123 <p>Status: COMPLETE</p>
 124
 125 <a name=ifconfig>
 126 <h1><a href=/hg/toybox/log/tip/toys/pending/ifconfig.c>ifconfig</a></h1>
 127
 128 <p>When ifconfig was submitted, it touched a half-dozen files. I glued it
 129 together into a single self-contained file, which needed a lot of
 130 cleanup. The final version is about 1/3 the size of the original.</p>
 131
 132 <ul>
 133 <li>old total: <a href=/hg/toybox/file/841/toys/pending/ifconfig.c>1504 lines (44268 bytes) in 38 functions</a></li>
 134 <li>new total: <a href=/hg/toybox/file/1113/toys/other/ifconfig.c>521 lines (15963 bytes) in 4 function</a></li>
 135 </ul>
 136
 137 <p>This was the first command I started cleaning up with the intent of
 138 describing the process, and especially the first few entries in this
 139 series describe many of the low hanging fruit techniques used to clean
 140 up a codebase.</p>
 141
 142 <ul>
 143 <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,
 144 unnecessary #defines, shorter code is easier to read.</a></li>
 145 <li>commit: <a href=/hg/toybox/rev/844>844</a>,
 146 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000881.html>Headers, replacing repeated code with loops,
 147 logical operators guaranteed to return 0 or 1, math on string constants,
 148 removing unnecessary variables and tests.</a></li>
 149 <li>commit: <a href=/hg/toybox/rev/852>852</a>,
 150 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000921.html>hg commit numbers, documenting the obvious, ordering
 151 to avoid prototypes, returning void, collate local declarations,
 152 use error_exit(), unnecessary parentheses, inline to remove variables/functions
 153 used only once, using *var instead of var[0], unnecessary typecasts,
 154 xprintf("\n") vs xputc('\n')</a></li>
 155 <li>commit: <a href=/hg/toybox/rev/856>856</a>,
 156 description: one line portability fix from Isaac Dunham</li>
 157 <li>commit: <a href=/hg/toybox/rev/861>861</a>
 158 and <a href=/hg/toybox/rev/863>863</a>,
 159 description:
 160 <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000910.html>Help
 161 infrastructure cleanup from Isaac Dunham</a>
 162 (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>
 163
 164 <li>commit: <a href=/hg/toybox/rev/862>862</a>,
 165 <a href=/hg/toybox/rev/864>864</a>,
 166 <a href=/hg/toybox/rev/866>866</a>: todo</li>
 167
 168 <li>commit: <a href=/hg/toybox/rev/869>869</a> and <a href=/hg/toybox/rev/870>870</a>,
 169 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000928.html>869:
 170 reorder to eliminate prototypes, put command_main() at end of file,
 171 870: long repeated variable prefixes, replacing struct+sscanf()+printf with a
 172 loop and a table (from iface_list, get_proc_info(), display_ifconfig()),
 173 use lib/xwrap.c functions to return void, why xstrcpy() fails closed,
 174 (functional comment: why multicast failed, CSLIP obsolecense), not being
 175 _too_ clever.</a></li>
 176 <li>commit: <a href=/hg/toybox/rev/878>878</a> and <a href=/hg/toybox/rev/879>879</a>:
 177 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!).
 178 879: inline three functions and simplify, some simplifications only show up
 179 after repeated inlining</a></li>
 180 <li>commit: <a href=/hg/toybox/rev/883>883</a>,
 181 description: move some common code to lib/ and posix headers to toys.h.</li>
 182 <li>commit: <a href=/hg/toybox/rev/898>898</a>,
 183 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
 184 an array, genericize - prefix logic, inline a use of set_flags().</a></li>
 185 <li>commit: <a href=/hg/toybox/rev/905>905</a>,
 186 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000992.html>remove unnecessary wrapper function, inlining more functions,
 187 relying on the values of constants that don't change across architectures
 188 (binary backwards compatability), more ifconfig_main table work,
 189 man ioctl_list.</a></li>
 190 <li>commit: <a href=/hg/toybox/rev/906>906</a>,
 191 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000994.html>More ifconfig_main() table work, remove vestigial arguments
 192 to functions, "a frenzy of inlining", slightly better error reporting,
 193 don't reinvent libc functions.</a></li>
 194 <li>commit: <a href=/hg/toybox/rev/907>907</a>,
 195 <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()
 196 and set_hw_address(), stop validating data we supplied, remove a table
 197 that's overkill (two entries, we don't even loop over it), when you don't need a
 198 NULL terminator for array, remove unnecessary memcpy(offsetof()) with
 199 assignment, trusting -funsigned-char.</a></li>
 200 <li>commit: <a href=/hg/toybox/rev/919>919</a>,
 201 <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-June/001027.html>todo whitespace damage, introduce IFREQ_OFFSZ() and poke() to
 202 ifconfig_main() table logic to fold more if/else parts into the table</a></li>
 203 <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>
 204 <li>commit: <a href=/hg/toybox/rev/921>921</a>, description: todo</li>
 205 <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>
 206 <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>
 207 <li>commit: <a href=/hg/toybox/rev/1104>1104</a>, description: todo</li>
 208 <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>
 209 <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>
 210 <li>commit: <a href=/hg/toybox/rev/1132>1132</a>, description: promotion from pending to other</li>
 211 <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>
 212 </ul>
 213
 214 <p>Status: COMPLETE.</p>
 215
 216 <h1><a href=/hg/toybox/log/tip/toys/pending/find.c>find</a></h1>
 217
 218 <pre>
 219 814 Initial version by Gurang Shastri.
 220 815
 221 816
 222 847 Felix Janda
 223 848 Whitespace (reindent from tabs -> 2 spaces)
 224 849 More cleanup
 225 867 Felix Janda Improve operator processing.
 226 874 Felix Janda
 227 875 Felix Janda
 228 887 Felix Janda fix -mtime
 229 </pre>
 230
 231 <ul>
 232 <li>commit: <a href=/hg/toybox/rev/849>849</a>,
 233 description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000895.html>here</a></li>
 234 </ul>
 235
 236 <p>Status: in progress.</p>
 237
 238 <h1><a href=/hg/toybox/log/917/toys/pending/stat.c>stat</a></h1>
 239
 240 <pre>
 241 747 initial submission
 242 810 whitespace
 243 811
 244 871 whitespace (reindent from 4 spaces to 2)
 245 872 Felix Janda cleanup
 246 885 Felix Janda
 247   move permission formatting (777 -> -rwxrwxrwx) from ls to lib so stat can reuse it.
 248 886 Felix Janda remove unimplemented options and clean up help text
 249 910 Felix Janda Add support for stating multiple files.
 250 911 Felix Janda Separate stat and statfs.
 251 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>
 252 <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001024.html>design pondering</a>
 253
 254  914 916
 255 </pre>
 256 <ul>
 257 <li>commit: <a href=/hg/toybox/rev/917>917</a></li>
 258 <li>commit: <a href=/hg/toybox/rev/918>918</a>,
 259 description: move to posix, default y.</li>
 260 </ul>
 261
 262 <p>Status: COMPLETE.</p>
 263
 264 <!--#include file="footer.html" -->