[PATCH] bug in coreutils/unlink.c

tito farmatito at tiscali.it
Sun Jun 22 20:43:16 UTC 2014


On Sunday 22 June 2014 20:44:48 Denys Vlasenko wrote:
> On Sunday 22 June 2014 16:45, Isaac Dunham wrote:
> > Attaching a revised patch, based on Tito's suggestions.
> > 
> > On Sun, Jun 22, 2014 at 08:11:25AM +0200, tito wrote:
> > > On Saturday 21 June 2014 23:01:10 Isaac Dunham wrote:
> > > > Here's an implementation of the "unlink" command.
> > > > Adds 252 bytes when enabled; a fair part is the error messages and text.
> > > 
> > > maybe it could be simplyfied, that way you get
> > > the error messages for free and also support
> > > -h to show usage text.
> > > This is untested as I am in a hurry now.
> > > 
> > > int unlink_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> > > int unlink_main(int argc, char **argv)
> > > {
> > >      
> > >       opt_complementary = "=1"; 	
> > >       
> > >       getopt32(argv, "");
> > > 
> > >       if (unlink(argv[1]))
> > >                bb_perror_msg_and_die("can't delete '%s'", argv[1]);
> > >       return 0;
> > > }
> > 
> > Shrinks .rodata by 30 bytes
> 
> We have xunlink().
> 
> > 
> > > 
> > > also the line:
> > > 
> > >  bb_perror_msg_and_die("can't delete '%s'", argv[0]);
> > > 
> > > should be:
> > > 
> > >  bb_perror_msg_and_die("can't delete '%s'", argv[1]);
> > Oops.
> > 
> > > Eventually instead of getopt32 you can use:
> > > 
> > > if (argc != 2)
> > > 	bb_show_usage();
> > 
> > Thanks; this also shrinks unlink_main by 22 bytes for a 200-byte total.
> > If I could do bb_show_trivial_usage() instead (just unlink_trivil_usage,
> > no full usage or "Busybox ..."), that would have seemed more appropriate.
> > But that doesn't exist, so...oh well.
> 
> Unfortunately, you were given bad advise. getopt32("") handles parameters such as
> "--" and "-notsupportedoptions".
> 
> I'm changing the code to:
> 
> int unlink_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> int unlink_main(int argc UNUSED_PARAM, char **argv)
> {
>         opt_complementary = "=1"; /* must have exactly 1 param */
>         getopt32(argv, "");
>         argv += optind;
>         xunlink(argv[0]);
>         return 0;
> }
> 
> Applied, thanks!

Hi,
sadly the applied version misbehaves with files starting with a dash:

echo > -test
ls -la
-rw-r--r--  1 tito tito      1 Jun 22 22:24 -test
./busybox unlink -test
unlink: invalid option -- 't'

even quoting does not help:

 ./busybox unlink "-test"
unlink: invalid option -- 't'
./busybox unlink '-test'
unlink: invalid option -- 't'

BTW.: also the original unlink program
shows this problem, maybe a bug
report should be filed upstream .

echo > -test
debian:~/Desktop/SourceCode/busybox$ unlink -test
unlink: invalid option -- 't'
Try `unlink --help' for more information.


I propose:

--- coreutils/unlink.c.orig     2014-06-22 22:28:45.934038033 +0200
+++ coreutils/unlink.c  2014-06-22 22:29:05.424761080 +0200
@@ -26,9 +26,8 @@
 int unlink_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int unlink_main(int argc UNUSED_PARAM, char **argv)
 {
-       opt_complementary = "=1"; /* must have exactly 1 param */
-       getopt32(argv, "");
-       argv += optind;
-       xunlink(argv[0]);
+       if (argc != 2)
+               bb_show_usage();
+       xunlink(argv[1]);
        return 0;
 }

With this patch applied unlink will try to unlink whatever filename
is passed as argv[1].

Ciao,
Tito
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unlink.patch
Type: text/x-patch
Size: 571 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20140622/4b50c0dc/attachment-0001.bin>


More information about the busybox mailing list