BusyBox Bug and Patch Tracking
BusyBox
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0003854 [BusyBox] Other minor always 06-24-08 21:36 06-25-08 13:55
Reporter qarce View Status public  
Assigned To BusyBox
Priority normal Resolution fixed  
Status closed   Product Version
Summary 0003854: bunzip2 and gzip don't honor the force option -f
Description
The -f option while parced has no effect on the tool

checking standard bunzip2 on my ubuntu system ... -f will remove the file before unpacking. I know the busybox code says this is not standard... but, why give a -f option if it's not really usable?


I am attaching a patch from my local repository based on busybox 1.9.0

I know your up to 1.10 or so but.... the bug still exists.
Additional Information Patch from my local svn repository:

Index: archival/bbunzip.c
===================================================================
--- archival/bbunzip.c (revision 34216)
+++ archival/bbunzip.c (working copy)
@@ -73,6 +73,17 @@
                                bb_error_msg("%s: unknown suffix - ignored", filename);
                                goto err;
                        }
+
+ /* if the force option is given unlink the file if it exists */
+ if ((stat(filename, &stat_buf) == 0) &&
+ (option_mask32 & OPT_FORCE)) {
+ if (unlink(filename) != 0) {
+ bb_simple_perror_msg(filename);
+ exitcode = 1;
+ goto free_name;
+ }
+ }
+
                        /* O_EXCL: "real" bunzip2 doesn't overwrite files */
                        /* GNU gunzip does not bail out, but goes to next file */
                        if (open_to_or_warn(STDOUT_FILENO, new_name, O_WRONLY | O_CREAT | O_EXCL,
@@ -163,6 +174,7 @@
 {
        getopt32(argv, "cfvdt");
        argv += optind;
+
        if (applet_name[2] == 'c')
                option_mask32 |= OPT_STDOUT;

@@ -261,10 +273,12 @@
 {
        getopt32(argv, "cfvdt");
        argv += optind;
+
        /* if called as zcat */
        if (applet_name[1] == 'c')
                option_mask32 |= OPT_STDOUT;

+
        return bbunpack(argv, make_new_name_gunzip, unpack_gunzip);
 }
Attached Files  busybox-1.9.0_bbunzip.patch [^] (981 bytes) 06-25-08 12:40

- Relationships

- Notes
(0008564)
qarce
06-25-08 12:41

Add a patch file which has a little better fix in it.
 
(0008574)
qarce
06-25-08 13:52

How do I go about getting this into busybox?

Do I need to do anything else?
 
(0008584)
vda
06-25-08 13:52

>why give a -f option if it's not really usable?

Because otherwise even scripts which used -f "just in case" would fail. Taking and ignoring an option sometimes is better than bombing out on it.

>I am attaching a patch

Thanks!

This is how we think in busybox land: why do we care whether unlink succeeded - the subsequent open will complain anyway. So we don't need to test unlink's result.

Now, since we ignore unlink's result, why do we care whether file exists before unlink? We can just do unlink blindly! The code:

                        if (option_mask32 & OPT_FORCE)
                                unlink(new_name);

That's all! :)

Thanks for bringing attention to the problem!
 
(0008594)
vda
06-25-08 13:55

Fixed in rev 22509, thanks.
 

- Issue History
Date Modified Username Field Change
06-24-08 21:36 qarce New Issue
06-24-08 21:36 qarce Status new => assigned
06-24-08 21:36 qarce Assigned To  => BusyBox
06-25-08 12:40 qarce File Added: busybox-1.9.0_bbunzip.patch
06-25-08 12:41 qarce Note Added: 0008564
06-25-08 13:52 qarce Note Added: 0008574
06-25-08 13:52 vda Note Added: 0008584
06-25-08 13:55 vda Status assigned => closed
06-25-08 13:55 vda Note Added: 0008594
06-25-08 13:55 vda Resolution open => fixed


Copyright © 2000 - 2006 Mantis Group
Powered by Mantis Bugtracker