Hello, I see a few problems, which can be summed up as follows: -the help is clunky and misplaced -the patch introduces a format-string vulnerability -the feature is not actually implemented; it skips rather than escaping
On Wed, Mar 09, 2016 at 02:51:26PM +0530, Sameer Pradhan wrote: --- a/toys/posix/ls.c 2016-02-26 13:23:36.000000000 +0530 +++ b/toys/posix/ls.c 2016-03-09 14:47:36.000000000 +0530 @@ -2,38 +2,38 @@ * * Copyright 2012 Andre Renaud <[email protected]> * Copyright 2012 Rob Landley <[email protected]> + * Copyright 2015 Sameer Prakash Pradhan <[email protected]> * * See http://opengroup.org/onlinepubs/9699919799/utilities/ls.html -USE_LS(NEWTOY(ls, USE_LS_COLOR("(color):;")"ZgoACFHLRSacdfhiklmnpqrstux1[-Cxm1][-Cxml][-Cxmo][-Cxmg][-cu][-ftS][-HL]", TOYFLAG_BIN|TOYFLAG_LOCALE)) +USE_LS(NEWTOY(ls, USE_LS_COLOR("(color):;")"ZgoACFHLRSab(escape)cdfhiklmnpqrstux1[-Cxm1][-Cxml][-Cxmo][-Cxmg][-cu][-ftS][-HL]", TOYFLAG_BIN|TOYFLAG_LOCALE)) config LS bool "ls" default y help - usage: ls [-ACFHLRSZacdfhiklmnpqrstux1] [directory...] - + usage: ls [-ACFHLRSZabcdfhiklmnpqrstux1] [directory...] list files what to show: - -a all files including .hidden -c use ctime for timestamps - -d directory, not contents -i inode number - -k block sizes in kilobytes -p put a '/' after dir names - -q unprintable chars as '?' -s size (in blocks) - -u use access time for timestamps -A list all files but . and .. - -H follow command line symlinks -L follow symlinks - -R recursively list files in subdirs -F append /dir *exe @sym |FIFO - -Z security context + -a all files including .hidden -b print C-style escapes for nongraphic characters (--escape) A few points here: 1-this message is *not* going to look nice; it will wrap around. It is also simultaneously verbose and unclear (does 'nongraphic' mean that it's not a box character? No, it means it's either nonprintable or has no visible output.) 2-"what to show" means the data, not the presentation. '-b' changes the presntation of non-printable characters (and ' '), so it should go under "output formats". Maybe add -b escape unprintable characters and space under output formats. Further, your re-ordering below is incorrect: '1' the number should precede all characters, and sorting it right before 'l' the letter is about the worst place to put it. output formats: - -1 list one file per line -C columns (sorted vertically) - -g like -l but no owner -h human readable sizes - -l long (show full details) -m comma separated - -n like -l but numeric uid/gid -o like -l but no group - -x columns (horizontal sort) + -C columns (sorted vertically) -g like -l but no owner + -h human readable sizes -1 list one file per line + -l long (show full details) -m comma separated + -n like -l but numeric uid/gid -o like -l but no group + -x columns (horizontal sort) sorting (default is alphabetical): - -f unsorted -r reverse -t timestamp -S size + -f unsorted -r reverse -t timestamp -S size config LS_COLOR bool "ls --color" @@ -293,7 +293,7 @@ if (-1 == dirfd) { strwidth(indir->name); - perror_msg_raw(indir->name); + perror_msg(indir->name); No, this is completely broken. indir->name, being the name on disk, is user data. It cannot be trusted. For example, try running mkdir -p test/ls && touch 'test/ls/a%s' && $LS test/ls/ where "$LS" is the appropriate invocation of toybox ls. return; } @@ -474,6 +474,11 @@ if (flags & FLAG_q) { char *p; for (p=sort[next]->name; *p; p++) fputc(isprint(*p) ? *p : '?', stdout); + } else if (flags & FLAG_b){ + char *b; + + for (b = sort[next]->name; *b; b++) + if (isgraph(*b)) fputc(*b, stdout); And I don't see where you're escaping anything. This would have to be something like this: for (b=...) { if (isgraph(*b)) fputc(*b, stdout); else printf("\\%3hho", *b); } Which will print C-style octal escapes for everything, rather than printing the standard \n, \r, \t ... escapes.
ls.c.patch
Description: Binary data
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
