[patch] New-applet howto. (and new applet)

Denis Vlasenko vda.linux at googlemail.com
Mon May 7 22:39:55 UTC 2007


On Monday 07 May 2007 13:14, Thomas Lundquist wrote:
> On Mon, May 07, 2007 at 01:07:38AM +0200, Denis Vlasenko wrote:
> > 
> > \t's invariably get incorrectly lined up without any fast way to see it.
> > Please read the comment on top of include/usage.h.
> 
> changed since I made the applet the first time, should have checked.
> 
> > busybox.h already includes most "usual" things. Drop #include <stdio.h> etc.
> 
> done, also added about it in the howto.
> 
> > You seem to delete crypt_make_salt() from passwd.c, but never add it anywhere.
> > Where is crypt_make_salt.c?
> 
> not added, done now.
> 
> > Well, why so many empty lines?
> 
> I find it easier to read but I've cut down a few.
> 
> new patch attached.

+#define cryptpw_trivial_usage \
+        "[-a des|md5] string"
+#define cryptpw_full_usage \
+        "Outputs a crypted version of the string.\n" \
+        "This also takes stdin as password\n" \
+        "\n\nOptions:\n" \
+        "      -a      Set algorithm for the password.\n" \
+        "              (Choices: des, md5. Default is md5.) //vda"

string is optional, thus should be [string].
"This also takes stdin as password\n" - what does this exactly mean?

+int cryptpw_main(int argc, char **argv)
+{
+       char *clear;
+       char *crypted;

crypted is used just once. do you really need this variable?

+       char salt[12]; /* "$N$XXXXXXXX" or "XX" */
+
+       unsigned opt;

opt is unused (well, assigned and never used after)

+       const char *opt_a = "";
+
+       int md5 = 1;    /* -a - password algorithm default: md5 */
+       int clen;
+
+       opt = getopt32(argc, argv, "a:", &opt_a);
+
+       /* move past the commandline options */
+       argc -= optind;

argc usage here and below can be eliminated

+       argv += optind;
+
+       if (!opt_a)
+               bb_show_usage();

Never going to happen.

+
+       if (strcasecmp(opt_a, "des") == 0) /* -a */
+               md5 = 0;

Can't you just compare there below instead of 'md5 == 1' check?
you don't need md5 variable then.

+
+       if (argc) {
+               clear = argv[0];
+       } else {
+               clear = xmalloc_fgets(stdin);
+               clen = strlen(clear);
+
+               if(clear[clen-1]=='\n') //vda
+                       clear[--clen] = '\0';

use xmalloc_getline and nuke clen.

+       }
+
+       crypt_make_salt(salt, 1); /* des */
+       if (md5 == 1) {
+               strcpy(salt, "$1$");
+               crypt_make_salt(salt + 3, 4);
+       }
+
+       crypted = pw_encrypt(clear, salt);
+       printf("%s\n", crypted);

Use puts() instead?

+
+       return (0);
+
+}

My version is attached.

Size: adding cryptpw yields:

# size busybox_old busybox_unstripped
   text    data     bss     dec     hex filename
 717644    2932   27472  748048   b6a10 busybox_old
 717908    2932   27472  748312   b6b18 busybox_unstripped

# make bloatcheck
function                                             old     new   delta
cryptpw_main                                           -     198    +198
.rodata                                           130867  130931     +64
packed_usage                                       22479   22509     +30
applets                                             3048    3060     +12
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 3/0 up/down: 304/0)             Total: 304 bytes

[heh, packed_usage counted twice (as itself and as rodata)]

Please review/test patch, and we can merget it. I like this applet.
--
vda
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 7.patch
Type: text/x-diff
Size: 12271 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20070508/4e7f86c4/attachment-0002.bin 


More information about the busybox mailing list