Rob Landley | b7ca39c | 2013-09-05 23:58:35 -0500 | [diff] [blame^] | 1 | <!--#include file="header.html" --> |
| 2 | |
| 3 | <h1>Cleaning up the toybox code.</h1> |
| 4 | |
| 5 | <p><a href=http://landley.net/notes.html#31-03-2013>Toybox |
| 6 | hasn't always taken proper advantage of external contributions</a>. |
| 7 | Lots of people want to help, but their contributions languish out of tree |
| 8 | or in the "pending" directory, awaiting cleanup.</p> |
| 9 | |
| 10 | <p>Toybox's design goals require simpler, tighter, and more explicit code |
| 11 | than most other implementations, among other reasons to allow better security |
| 12 | auditing. Writing "another" implementation of standard command line tools |
| 13 | isn't very interesting, they should be _better_ implementations. |
| 14 | Unfortunately, this means existing, working commands often take more effort to |
| 15 | clean up to Toybox's standards than writing a new one from scratch, not |
| 16 | because they don't work, but because we require an unusual level of polish.</p> |
| 17 | |
| 18 | <p>In hopes of teaching more people how to do this |
| 19 | cleanup work, I've started breaking cleanup changes into smaller chunks and |
| 20 | posting explanations of each change to the mailing list. |
| 21 | Below are indexes of each cleanup series. Line/byte totals of completed |
| 22 | series are given for scale, but the point of this work is simplicity, not |
| 23 | size.</p> |
| 24 | |
| 25 | <p>(Note: mercurial's web viewer doesn't follow renames, so the commit list |
| 26 | links may not show the final version of each file moved from the "pending" |
| 27 | directory to its final location. The summaries link the initial and cleaned |
| 28 | versions of each file when giving line counts.)</p> |
| 29 | |
| 30 | <hr> |
| 31 | |
| 32 | <p>General advice and/or policy:</p> |
| 33 | |
| 34 | <p><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html>Error messages and internationalization.</a></p> |
| 35 | |
| 36 | <p><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000891.html>Why not "const"?</a></p> |
| 37 | |
| 38 | <p>(I leave the <a href=http://lkml.indiana.edu/hypermail/linux/kernel/1308.3/03890.html>Why not "bool"?</a> explanation to Linus Torvalds.)</p> |
| 39 | |
| 40 | <p><a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000893.html>Rationale for removing debug code</a>, the interesting bit says:</p> |
| 41 | <blockquote><p>Rule of thumb: other people's debugging printfs are seldom as |
| 42 | useful as your own debugging printfs. Because you know what your output |
| 43 | _means_, and if you can't annotate the code to dump what you need you haven't |
| 44 | read enough of it yet. |
| 45 | </p></blockquote> |
| 46 | |
| 47 | <hr> |
| 48 | |
| 49 | <h1><a href=/hg/toybox/log/900/toys/pending/uuencode.c>uuencode</a> and |
| 50 | <a href=http:/hg/toybox/log/900/toys/pending/uudecode.c>uudecode</a> |
| 51 | </h1> |
| 52 | |
| 53 | <p>This is an example of cleaning up something |
| 54 | that started in a condition most projects would be quite happy with. |
| 55 | The initial submission of uuencode and uudecode was a good high |
| 56 | quality contribution, written by a seasoned developer who did an excellent |
| 57 | job. It was still possible to shrink uuencode almost by half:</p> |
| 58 | |
| 59 | <ul> |
| 60 | <li>old total: <a href=/hg/toybox/file/828/toys/pending/uuencode.c>116 lines (2743 |
| 61 | bytes) in 7 functions</a></li> |
| 62 | <li>new total: <a href=/hg/toybox/file/841/toys/posix/uuencode.c>67 lines (1440 |
| 63 | bytes) in 1 function</a></li> |
| 64 | </ul> |
| 65 | |
| 66 | <ul> |
| 67 | <li>commit: <a href=/hg/toybox/rev/830>830</a>, |
| 68 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000904.html>part 1</a> and |
| 69 | <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000903.html>part 2</a></li> |
| 70 | <li>commit: <a href=/hg/toybox/rev/831>831</a>, |
| 71 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000919.html>inline, default Y, move to toys/posix</a></li> |
| 72 | <li>commit: <a href=/hg/toybox/rev/837>837</a>, |
| 73 | description: test suite.</li> |
| 74 | </ul> |
| 75 | |
| 76 | <p>Status: COMPLETE</p> |
| 77 | |
| 78 | <p>I tried to do the uudecode cleanup in smaller stages:</p> |
| 79 | |
| 80 | <ul> |
| 81 | <li>old: <a href=/hg/toybox/file/828/toys/pending/uudecode.c>175 |
| 82 | lines (4534 bytes) in 9 functions</a></li> |
| 83 | <li>new: <a href=/hg/toybox/file/840/toys/posix/uudecode.c>107 lines |
| 84 | (2300 bytes) in 1 function</a></li> |
| 85 | </ul> |
| 86 | |
| 87 | <ul> |
| 88 | <li>commit: <a href=/hg/toybox/rev/833>833</a>, |
| 89 | description: preparatory adjustments to test suite.</li> |
| 90 | <li>commit: <a href=/hg/toybox/rev/835>835</a>, |
| 91 | description: todo</a></li> |
| 92 | <li>commit: <a href=/hg/toybox/rev/838>838</a>, |
| 93 | description: todo</a></li> |
| 94 | <li>commit: <a href=/hg/toybox/rev/839>839</a>, |
| 95 | description: todo</a></li> |
| 96 | <li>commit: <a href=/hg/toybox/rev/839>839</a>, |
| 97 | description: todo (finish, move pending->posix, default y)</a></li> |
| 98 | </ul> |
| 99 | |
| 100 | <p>Status: COMPLETE</p> |
| 101 | |
| 102 | <h1><a href=/hg/toybox/log/tip/toys/pending/ifconfig.c>ifconfig</a></h1> |
| 103 | |
| 104 | <p>The largest (ongoing) documented cleanup to date, still in progress:</p> |
| 105 | |
| 106 | <ul> |
| 107 | <li>commit: <a href=/hg/toybox/rev/843>843</a>, |
| 108 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000882.html>Explaining the ifconfig cleanup: part I.</a></li> |
| 109 | <li>commit: <a href=/hg/toybox/rev/844>844</a>, |
| 110 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000881.html>Explaining more ifconfig cleanup.</a></li> |
| 111 | <li>commit: <a href=/hg/toybox/rev/852>852</a>, |
| 112 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000921.html>cleanup</a></li> |
| 113 | <li>commit: <a href=/hg/toybox/rev/856>856</a>, |
| 114 | description: A one line portability fix from Isaac Dunham</li> |
| 115 | <li>commit: <a href=/hg/toybox/rev/861>861</a> |
| 116 | and <a href=/hg/toybox/rev/863>863</a>, |
| 117 | description: |
| 118 | <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000910.html>Help |
| 119 | infrastructure cleanup from Isaac Dunham</a> |
| 120 | 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> |
| 121 | |
| 122 | <li>commit: <a href=/hg/toybox/rev/862>862</a>, |
| 123 | <a href=/hg/toybox/rev/864>864</a>, |
| 124 | <a href=/hg/toybox/rev/866>866</a>: todo</li> |
| 125 | |
| 126 | <li>commit: <a href=/hg/toybox/rev/869>869</a> and <a href=/hg/toybox/rev/870>870</a>, |
| 127 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000928.html>here</a></li> |
| 128 | <li>commit: <a href=/hg/toybox/rev/878>878</a> and <a href=/hg/toybox/rev/879>879</a>: |
| 129 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000946.html>here</a></li> |
| 130 | <li>commit: <a href=/hg/toybox/rev/883>883</a>, |
| 131 | description: todo</li> |
| 132 | <li>commit: <a href=/hg/toybox/rev/898>898</a>, |
| 133 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000974.html>here</a></li> |
| 134 | <li>commit: <a href=/hg/toybox/rev/905>905</a>, |
| 135 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000992.html>here</a></li> |
| 136 | <li>commit: <a href=/hg/toybox/rev/906>906</a>, |
| 137 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000994.html>here</a></li> |
| 138 | <li>commit: <a href=/hg/toybox/rev/907>907</a>, |
| 139 | <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/000995.html>here</a></li> |
| 140 | </ul> |
| 141 | |
| 142 | <p>Status: in progress.</p> |
| 143 | |
| 144 | <h1><a href=/hg/toybox/log/tip/toys/pending/find.c>find</a></h1> |
| 145 | |
| 146 | <pre> |
| 147 | 814 Initial version by Gurang Shastri. |
| 148 | 815 |
| 149 | 816 |
| 150 | 847 Felix Janda |
| 151 | 848 Whitespace (reindent from tabs -> 2 spaces) |
| 152 | 849 More cleanup |
| 153 | 867 Felix Janda Improve operator processing. |
| 154 | 874 Felix Janda |
| 155 | 875 Felix Janda |
| 156 | 887 Felix Janda fix -mtime |
| 157 | </pre> |
| 158 | |
| 159 | <ul> |
| 160 | <li>commit: <a href=/hg/toybox/rev/849>849</a>, |
| 161 | description: <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000895.html>here</a></li> |
| 162 | </ul> |
| 163 | |
| 164 | <p>Status: in progress.</p> |
| 165 | |
| 166 | <h1><a href=/hg/toybox/log/917/toys/pending/stat.c>stat</a></h1> |
| 167 | |
| 168 | <pre> |
| 169 | 747 initial submission |
| 170 | 810 whitespace |
| 171 | 811 |
| 172 | 871 whitespace (reindent from 4 spaces to 2) |
| 173 | 872 Felix Janda cleanup |
| 174 | 885 Felix Janda |
| 175 | move permission formatting (777 -> -rwxrwxrwx) from ls to lib so stat can reuse it. |
| 176 | 886 Felix Janda remove unimplemented options and clean up help text |
| 177 | 910 Felix Janda Add support for stating multiple files. |
| 178 | 911 Felix Janda Separate stat and statfs. |
| 179 | 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> |
| 180 | <a href=http://lists.landley.net/pipermail/toybox-landley.net/2013-May/001024.html>design pondering</a> |
| 181 | |
| 182 | 914 916 |
| 183 | </pre> |
| 184 | <ul> |
| 185 | <li>commit: <a href=/hg/toybox/rev/917>917</a></li> |
| 186 | <li>commit: <a href=/hg/toybox/rev/918>918</a>, |
| 187 | description: move to posix, default y.</li> |
| 188 | </ul> |
| 189 | |
| 190 | <p>Status: COMPLETE.</p> |
| 191 | |
| 192 | <!--#include file="footer.html" --> |