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