tar bugfix n.547 (patch submission)

Rob Landley rob at landley.net
Mon Jan 9 03:09:51 UTC 2006


On Tuesday 27 December 2005 04:10, Roberto A. Foglietta wrote:
> Hi to all,
>
>   I submited another patch fot this issue:
>
>   0000547: tar archive corruption when packing unreadable files
>            http://busybox.net/bugs/view.php?id=547

Ok, poking at the patch:

 if ((tbInfo->hlInfo == NULL)
-  && (S_ISREG(statbuf->st_mode))) {
+ && (S_ISREG(statbuf->st_mode))) {
   int inputFileFd;

Any reason for the indentation change?  The second line is a continuation of 
the first, obviously _we_ thought it should therefore be indented...

Also, copying writeTarHeader from one place to two isn't exaclty a code 
shrinkage.  Have to look at the actual file in context to see if there's a 
graceful way around that...

 /* Hang up the tools, close up shop, head home */
  close(tbInfo.tarFd);
- if (errorFlag)
-  bb_error_msg("Error exit delayed from previous errors");
+ //if (errorFlag)
+ // bb_error_msg("Error exit delayed from previous errors");
 
If this is intentional, just delete it.  If it's not let me know so that 
doesn't go in.

 
-  if ((tar_handle->action_header == header_list) ||
-    (tar_handle->action_header == header_verbose_list)) {
+  if ((tar_handle->action_header == header_list) 
+  ||  (tar_handle->action_header == header_verbose_list)) {
    verboseFlag = TRUE;

This is another unrelated rewordwrapping that breaks the indentation (again).  
Not exactly a clean patch with no superflous changes, and not large enough 
change to justify other unrelated things this close to a release.
 
So, having reviewed the patch I _think_ the only actual change is moving (and 
duplicating) writeTarHeader()...

You know, if we were _really_ sick people, we'd mmap the file and just write() 
from there as one big block of data.  (Ok, wouldn't work too well for long 
files on 32-bit systems, but in terms of runtime memory usage it might be a 
win.  Or might not.)  Not looking at it right now, of course.

Ok, svn 13189 should fix it.  Can you confirm it's ok?

Rob
-- 
Steve Ballmer: Innovation!  Inigo Montoya: You keep using that word.
I do not think it means what you think it means.



More information about the busybox mailing list