A sweep of the sparse code prompted by a bug report by Jim Meyering.

* src/sparse.c: Include <inttostr.h>.
(struct tar_sparse_file): offset and dumped_size are off_t, not
size_t.  optab is now const *.
(dump_zeros): Return bool success flag, not off_t.
All callers changed.
Use a constant-zero buffer rather than clearing a buffer each time.
Don't mess up if write fails.
(dump_zeros, check_sparse_region):
Don't assume off_t is no wider than size_t.
(tar_sparse_init): Don't bother clearing a field that is already clear.
(zero_block_p): First arg is const *, not *.
(clear_block, SPARSES_INIT_COUNT): Remove.
(sparse_add_map): First arg is now struct start_stat_info *, not
struct tar_sparse_file *.  All callers changed.
Use x2nrealloc to check for size_t overflow.
(parse_scan_file): Cache commonly-used parts of file.
Use an auto buffer, not a static one.
Don't bother clearing the buffer; not needed.
Don't bother clearing items that are already clear.
(oldgnu_optab, star_optab, pax_optab): Now const.
(sparse_dump_region): Don't bother clearing the buffer before
reading into it; just clear the parts that aren't read into.
(sparse_dump_file): Clear the whole local variable 'file'.
(diff_buffer): Remove; now a local var.
(check_sparse_region): Don't bother clearing buffer before
reading into it.  Don't assume off_t is promoted to long.
(oldgnu_get_sparse_info, star_get_sparse_info):
Use an auto status, not static.
* src/tar.h (struct tar_stat_info): had_trailing_slash is
now bool, not int.
* src/xheader.c (sparse_offset_coder, sparse_numbytes_coder):
Rewrite to avoid cast.
(sparse_offset_decoder, sparse_numbytes_decoder):
Diagnose excess entries rather than crashing.
This commit is contained in:
Paul Eggert
2005-06-23 06:55:01 +00:00
parent 4f4a858eb6
commit 040b5ab5f9
4 changed files with 123 additions and 102 deletions

View File

@@ -1,3 +1,41 @@
2005-06-22 Paul Eggert <eggert@cs.ucla.edu>
A sweep of the sparse code prompted by a bug report by Jim Meyering.
* src/sparse.c: Include <inttostr.h>.
(struct tar_sparse_file): offset and dumped_size are off_t, not
size_t. optab is now const *.
(dump_zeros): Return bool success flag, not off_t.
All callers changed.
Use a constant-zero buffer rather than clearing a buffer each time.
Don't mess up if write fails.
(dump_zeros, check_sparse_region):
Don't assume off_t is no wider than size_t.
(tar_sparse_init): Don't bother clearing a field that is already clear.
(zero_block_p): First arg is const *, not *.
(clear_block, SPARSES_INIT_COUNT): Remove.
(sparse_add_map): First arg is now struct start_stat_info *, not
struct tar_sparse_file *. All callers changed.
Use x2nrealloc to check for size_t overflow.
(parse_scan_file): Cache commonly-used parts of file.
Use an auto buffer, not a static one.
Don't bother clearing the buffer; not needed.
Don't bother clearing items that are already clear.
(oldgnu_optab, star_optab, pax_optab): Now const.
(sparse_dump_region): Don't bother clearing the buffer before
reading into it; just clear the parts that aren't read into.
(sparse_dump_file): Clear the whole local variable 'file'.
(diff_buffer): Remove; now a local var.
(check_sparse_region): Don't bother clearing buffer before
reading into it. Don't assume off_t is promoted to long.
(oldgnu_get_sparse_info, star_get_sparse_info):
Use an auto status, not static.
* src/tar.h (struct tar_stat_info): had_trailing_slash is
now bool, not int.
* src/xheader.c (sparse_offset_coder, sparse_numbytes_coder):
Rewrite to avoid cast.
(sparse_offset_decoder, sparse_numbytes_decoder):
Diagnose excess entries rather than crashing.
2005-06-22 Jim Meyering <jim@meyering.net> 2005-06-22 Jim Meyering <jim@meyering.net>
* src/common.h (timespec_lt): Add a return type: bool. * src/common.h (timespec_lt): Add a return type: bool.

View File

@@ -17,6 +17,7 @@
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */
#include <system.h> #include <system.h>
#include <inttostr.h>
#include <quotearg.h> #include <quotearg.h>
#include "common.h" #include "common.h"
@@ -47,47 +48,47 @@ struct tar_sparse_file
{ {
int fd; /* File descriptor */ int fd; /* File descriptor */
bool seekable; /* Is fd seekable? */ bool seekable; /* Is fd seekable? */
size_t offset; /* Current offset in fd if seekable==false. off_t offset; /* Current offset in fd if seekable==false.
Otherwise unused */ Otherwise unused */
size_t dumped_size; /* Number of bytes actually written off_t dumped_size; /* Number of bytes actually written
to the archive */ to the archive */
struct tar_stat_info *stat_info; /* Information about the file */ struct tar_stat_info *stat_info; /* Information about the file */
struct tar_sparse_optab *optab; struct tar_sparse_optab const *optab;
void *closure; /* Any additional data optab calls might void *closure; /* Any additional data optab calls might
reqiure */ require */
}; };
/* Dump zeros to file->fd until offset is reached. It is used instead of /* Dump zeros to file->fd until offset is reached. It is used instead of
lseek if the output file is not seekable */ lseek if the output file is not seekable */
static long static bool
dump_zeros (struct tar_sparse_file *file, off_t offset) dump_zeros (struct tar_sparse_file *file, off_t offset)
{ {
char buf[BLOCKSIZE]; static char const zero_buf[BLOCKSIZE];
if (offset - file->offset < 0) if (offset < file->offset)
{ {
errno = EINVAL; errno = EINVAL;
return -1; return false;
} }
memset (buf, 0, sizeof buf);
while (file->offset < offset) while (file->offset < offset)
{ {
size_t size = offset - file->offset; size_t size = (BLOCKSIZE < offset - file->offset
size_t wrbytes; ? BLOCKSIZE
: offset - file->offset);
if (size > sizeof buf) ssize_t wrbytes;
size = sizeof buf;
wrbytes = write (file->fd, buf, size); wrbytes = write (file->fd, zero_buf, size);
if (wrbytes <= 0) if (wrbytes <= 0)
{ {
if (wrbytes == 0) if (wrbytes == 0)
errno = EINVAL; errno = EINVAL;
return -1; return false;
} }
file->offset += wrbytes; file->offset += wrbytes;
} }
return file->offset;
return true;
} }
static bool static bool
@@ -101,7 +102,6 @@ tar_sparse_member_p (struct tar_sparse_file *file)
static bool static bool
tar_sparse_init (struct tar_sparse_file *file) tar_sparse_init (struct tar_sparse_file *file)
{ {
file->dumped_size = 0;
if (file->optab->init) if (file->optab->init)
return file->optab->init (file); return file->optab->init (file);
return true; return true;
@@ -168,14 +168,9 @@ tar_sparse_fixup_header (struct tar_sparse_file *file)
static bool static bool
lseek_or_error (struct tar_sparse_file *file, off_t offset) lseek_or_error (struct tar_sparse_file *file, off_t offset)
{ {
off_t off; if (file->seekable
? lseek (file->fd, offset, SEEK_SET) < 0
if (file->seekable) : ! dump_zeros (file, offset))
off = lseek (file->fd, offset, SEEK_SET);
else
off = dump_zeros (file, offset);
if (off < 0)
{ {
seek_diag_details (file->stat_info->orig_file_name, offset); seek_diag_details (file->stat_info->orig_file_name, offset);
return false; return false;
@@ -187,7 +182,7 @@ lseek_or_error (struct tar_sparse_file *file, off_t offset)
it's made *entirely* of zeros, returning a 0 the instant it finds it's made *entirely* of zeros, returning a 0 the instant it finds
something that is a nonzero, i.e., useful data. */ something that is a nonzero, i.e., useful data. */
static bool static bool
zero_block_p (char *buffer, size_t size) zero_block_p (char const *buffer, size_t size)
{ {
while (size--) while (size--)
if (*buffer++) if (*buffer++)
@@ -195,58 +190,44 @@ zero_block_p (char *buffer, size_t size)
return true; return true;
} }
#define clear_block(p) memset (p, 0, BLOCKSIZE);
#define SPARSES_INIT_COUNT SPARSES_IN_SPARSE_HEADER
static void static void
sparse_add_map (struct tar_sparse_file *file, struct sp_array *sp) sparse_add_map (struct tar_stat_info *st, struct sp_array const *sp)
{ {
if (file->stat_info->sparse_map == NULL) struct sp_array *sparse_map = st->sparse_map;
{ size_t avail = st->sparse_map_avail;
file->stat_info->sparse_map = if (avail == st->sparse_map_size)
xmalloc (SPARSES_INIT_COUNT * sizeof file->stat_info->sparse_map[0]); st->sparse_map = sparse_map =
file->stat_info->sparse_map_size = SPARSES_INIT_COUNT; x2nrealloc (sparse_map, &st->sparse_map_size, sizeof *sparse_map);
} sparse_map[avail] = *sp;
else if (file->stat_info->sparse_map_avail == file->stat_info->sparse_map_size) st->sparse_map_avail = avail + 1;
{
file->stat_info->sparse_map_size *= 2;
file->stat_info->sparse_map =
xrealloc (file->stat_info->sparse_map,
file->stat_info->sparse_map_size
* sizeof file->stat_info->sparse_map[0]);
}
file->stat_info->sparse_map[file->stat_info->sparse_map_avail++] = *sp;
} }
/* Scan the sparse file and create its map */ /* Scan the sparse file and create its map */
static bool static bool
sparse_scan_file (struct tar_sparse_file *file) sparse_scan_file (struct tar_sparse_file *file)
{ {
static char buffer[BLOCKSIZE]; struct tar_stat_info *st = file->stat_info;
int fd = file->fd;
char buffer[BLOCKSIZE];
size_t count; size_t count;
off_t offset = 0; off_t offset = 0;
struct sp_array sp = {0, 0}; struct sp_array sp = {0, 0};
if (!lseek_or_error (file, 0)) if (!lseek_or_error (file, 0))
return false; return false;
clear_block (buffer);
file->stat_info->sparse_map_avail = 0;
file->stat_info->archive_file_size = 0;
if (!tar_sparse_scan (file, scan_begin, NULL)) if (!tar_sparse_scan (file, scan_begin, NULL))
return false; return false;
while ((count = safe_read (file->fd, buffer, sizeof buffer)) != 0 while ((count = safe_read (fd, buffer, sizeof buffer)) != 0
&& count != SAFE_READ_ERROR) && count != SAFE_READ_ERROR)
{ {
/* Analize the block */ /* Analyze the block. */
if (zero_block_p (buffer, count)) if (zero_block_p (buffer, count))
{ {
if (sp.numbytes) if (sp.numbytes)
{ {
sparse_add_map (file, &sp); sparse_add_map (st, &sp);
sp.numbytes = 0; sp.numbytes = 0;
if (!tar_sparse_scan (file, scan_block, NULL)) if (!tar_sparse_scan (file, scan_block, NULL))
return false; return false;
@@ -257,26 +238,25 @@ sparse_scan_file (struct tar_sparse_file *file)
if (sp.numbytes == 0) if (sp.numbytes == 0)
sp.offset = offset; sp.offset = offset;
sp.numbytes += count; sp.numbytes += count;
file->stat_info->archive_file_size += count; st->archive_file_size += count;
if (!tar_sparse_scan (file, scan_block, buffer)) if (!tar_sparse_scan (file, scan_block, buffer))
return false; return false;
} }
offset += count; offset += count;
clear_block (buffer);
} }
if (sp.numbytes == 0) if (sp.numbytes == 0)
sp.offset = offset; sp.offset = offset;
sparse_add_map (file, &sp); sparse_add_map (st, &sp);
file->stat_info->archive_file_size += count; st->archive_file_size += count;
return tar_sparse_scan (file, scan_end, NULL); return tar_sparse_scan (file, scan_end, NULL);
} }
static struct tar_sparse_optab oldgnu_optab; static struct tar_sparse_optab const oldgnu_optab;
static struct tar_sparse_optab star_optab; static struct tar_sparse_optab const star_optab;
static struct tar_sparse_optab pax_optab; static struct tar_sparse_optab const pax_optab;
static bool static bool
sparse_select_optab (struct tar_sparse_file *file) sparse_select_optab (struct tar_sparse_file *file)
@@ -321,18 +301,18 @@ sparse_dump_region (struct tar_sparse_file *file, size_t i)
size_t bytes_read; size_t bytes_read;
blk = find_next_block (); blk = find_next_block ();
memset (blk->buffer, 0, BLOCKSIZE);
bytes_read = safe_read (file->fd, blk->buffer, bufsize); bytes_read = safe_read (file->fd, blk->buffer, bufsize);
if (bytes_read == SAFE_READ_ERROR) if (bytes_read == SAFE_READ_ERROR)
{ {
read_diag_details (file->stat_info->orig_file_name, read_diag_details (file->stat_info->orig_file_name,
file->stat_info->sparse_map[i].offset (file->stat_info->sparse_map[i].offset
+ file->stat_info->sparse_map[i].numbytes + file->stat_info->sparse_map[i].numbytes
- bytes_left, - bytes_left),
bufsize); bufsize);
return false; return false;
} }
memset (blk->buffer + bytes_read, 0, BLOCKSIZE - bytes_read);
bytes_left -= bytes_read; bytes_left -= bytes_read;
file->dumped_size += bytes_read; file->dumped_size += bytes_read;
set_next_block_after (blk); set_next_block_after (blk);
@@ -389,13 +369,12 @@ enum dump_status
sparse_dump_file (int fd, struct tar_stat_info *st) sparse_dump_file (int fd, struct tar_stat_info *st)
{ {
bool rc; bool rc;
struct tar_sparse_file file; struct tar_sparse_file file = { 0, };
file.stat_info = st; file.stat_info = st;
file.fd = fd; file.fd = fd;
file.seekable = true; /* File *must* be seekable for dump to work */ file.seekable = true; /* File *must* be seekable for dump to work */
file.offset = 0;
if (!sparse_select_optab (&file) if (!sparse_select_optab (&file)
|| !tar_sparse_init (&file)) || !tar_sparse_init (&file))
return dump_status_not_implemented; return dump_status_not_implemented;
@@ -414,7 +393,7 @@ sparse_dump_file (int fd, struct tar_stat_info *st)
} }
} }
pad_archive(file.stat_info->archive_file_size - file.dumped_size); pad_archive (file.stat_info->archive_file_size - file.dumped_size);
return (tar_sparse_done (&file) && rc) ? dump_status_ok : dump_status_short; return (tar_sparse_done (&file) && rc) ? dump_status_ok : dump_status_short;
} }
@@ -460,7 +439,7 @@ sparse_extract_file (int fd, struct tar_stat_info *st, off_t *size)
file.fd = fd; file.fd = fd;
file.seekable = lseek (fd, 0, SEEK_SET) == 0; file.seekable = lseek (fd, 0, SEEK_SET) == 0;
file.offset = 0; file.offset = 0;
if (!sparse_select_optab (&file) if (!sparse_select_optab (&file)
|| !tar_sparse_init (&file)) || !tar_sparse_init (&file))
return dump_status_not_implemented; return dump_status_not_implemented;
@@ -491,8 +470,6 @@ sparse_skip_file (struct tar_stat_info *st)
} }
static char diff_buffer[BLOCKSIZE];
static bool static bool
check_sparse_region (struct tar_sparse_file *file, off_t beg, off_t end) check_sparse_region (struct tar_sparse_file *file, off_t beg, off_t end)
{ {
@@ -502,11 +479,9 @@ check_sparse_region (struct tar_sparse_file *file, off_t beg, off_t end)
while (beg < end) while (beg < end)
{ {
size_t bytes_read; size_t bytes_read;
size_t rdsize = end - beg; size_t rdsize = BLOCKSIZE < end - beg ? BLOCKSIZE : end - beg;
char diff_buffer[BLOCKSIZE];
if (rdsize > BLOCKSIZE)
rdsize = BLOCKSIZE;
clear_block (diff_buffer);
bytes_read = safe_read (file->fd, diff_buffer, rdsize); bytes_read = safe_read (file->fd, diff_buffer, rdsize);
if (bytes_read == SAFE_READ_ERROR) if (bytes_read == SAFE_READ_ERROR)
{ {
@@ -517,8 +492,10 @@ check_sparse_region (struct tar_sparse_file *file, off_t beg, off_t end)
} }
if (!zero_block_p (diff_buffer, bytes_read)) if (!zero_block_p (diff_buffer, bytes_read))
{ {
char begbuf[INT_BUFSIZE_BOUND (off_t)];
report_difference (file->stat_info, report_difference (file->stat_info,
_("File fragment at %lu is not a hole"), beg); _("File fragment at %s is not a hole"),
offtostr (beg, begbuf));
return false; return false;
} }
@@ -539,6 +516,7 @@ check_data_region (struct tar_sparse_file *file, size_t i)
{ {
size_t bytes_read; size_t bytes_read;
size_t rdsize = (size_left > BLOCKSIZE) ? BLOCKSIZE : size_left; size_t rdsize = (size_left > BLOCKSIZE) ? BLOCKSIZE : size_left;
char diff_buffer[BLOCKSIZE];
union block *blk = find_next_block (); union block *blk = find_next_block ();
if (!blk) if (!blk)
@@ -551,9 +529,9 @@ check_data_region (struct tar_sparse_file *file, size_t i)
if (bytes_read == SAFE_READ_ERROR) if (bytes_read == SAFE_READ_ERROR)
{ {
read_diag_details (file->stat_info->orig_file_name, read_diag_details (file->stat_info->orig_file_name,
file->stat_info->sparse_map[i].offset (file->stat_info->sparse_map[i].offset
+ file->stat_info->sparse_map[i].numbytes + file->stat_info->sparse_map[i].numbytes
- size_left, - size_left),
rdsize); rdsize);
return false; return false;
} }
@@ -647,7 +625,7 @@ oldgnu_add_sparse (struct tar_sparse_file *file, struct sparse *s)
|| file->stat_info->archive_file_size < 0) || file->stat_info->archive_file_size < 0)
return add_fail; return add_fail;
sparse_add_map (file, &sp); sparse_add_map (file->stat_info, &sp);
return add_ok; return add_ok;
} }
@@ -658,7 +636,7 @@ oldgnu_fixup_header (struct tar_sparse_file *file)
which actually contains archived size. The following fixes it */ which actually contains archived size. The following fixes it */
file->stat_info->archive_file_size = file->stat_info->stat.st_size; file->stat_info->archive_file_size = file->stat_info->stat.st_size;
file->stat_info->stat.st_size = file->stat_info->stat.st_size =
OFF_FROM_HEADER (current_header->oldgnu_header.realsize); OFF_FROM_HEADER (current_header->oldgnu_header.realsize);
return true; return true;
} }
@@ -669,7 +647,7 @@ oldgnu_get_sparse_info (struct tar_sparse_file *file)
size_t i; size_t i;
union block *h = current_header; union block *h = current_header;
int ext_p; int ext_p;
static enum oldgnu_add_status rc; enum oldgnu_add_status rc;
file->stat_info->sparse_map_avail = 0; file->stat_info->sparse_map_avail = 0;
for (i = 0; i < SPARSES_IN_OLDGNU_HEADER; i++) for (i = 0; i < SPARSES_IN_OLDGNU_HEADER; i++)
@@ -756,7 +734,7 @@ oldgnu_dump_header (struct tar_sparse_file *file)
return true; return true;
} }
static struct tar_sparse_optab oldgnu_optab = { static struct tar_sparse_optab const oldgnu_optab = {
NULL, /* No init function */ NULL, /* No init function */
NULL, /* No done function */ NULL, /* No done function */
oldgnu_sparse_member_p, oldgnu_sparse_member_p,
@@ -795,7 +773,7 @@ star_get_sparse_info (struct tar_sparse_file *file)
size_t i; size_t i;
union block *h = current_header; union block *h = current_header;
int ext_p; int ext_p;
static enum oldgnu_add_status rc; enum oldgnu_add_status rc = add_ok;
file->stat_info->sparse_map_avail = 0; file->stat_info->sparse_map_avail = 0;
@@ -837,7 +815,7 @@ star_get_sparse_info (struct tar_sparse_file *file)
} }
static struct tar_sparse_optab star_optab = { static struct tar_sparse_optab const star_optab = {
NULL, /* No init function */ NULL, /* No init function */
NULL, /* No done function */ NULL, /* No done function */
star_sparse_member_p, star_sparse_member_p,
@@ -890,7 +868,7 @@ pax_dump_header (struct tar_sparse_file *file)
return true; return true;
} }
static struct tar_sparse_optab pax_optab = { static struct tar_sparse_optab const pax_optab = {
NULL, /* No init function */ NULL, /* No init function */
NULL, /* No done function */ NULL, /* No done function */
pax_sparse_member_p, pax_sparse_member_p,
@@ -901,4 +879,3 @@ static struct tar_sparse_optab pax_optab = {
sparse_dump_region, sparse_dump_region,
sparse_extract_region, sparse_extract_region,
}; };

View File

@@ -273,7 +273,7 @@ struct tar_stat_info
char *orig_file_name; /* name of file read from the archive header */ char *orig_file_name; /* name of file read from the archive header */
char *file_name; /* name of file for the current archive entry char *file_name; /* name of file for the current archive entry
after being normalized. */ after being normalized. */
int had_trailing_slash; /* nonzero if the current archive entry had a bool had_trailing_slash; /* true if the current archive entry had a
trailing slash before it was normalized. */ trailing slash before it was normalized. */
char *link_name; /* name of link for the current archive entry. */ char *link_name; /* name of link for the current archive entry. */

View File

@@ -1079,8 +1079,8 @@ static void
sparse_offset_coder (struct tar_stat_info const *st, char const *keyword, sparse_offset_coder (struct tar_stat_info const *st, char const *keyword,
struct xheader *xhdr, void *data) struct xheader *xhdr, void *data)
{ {
size_t i = *(size_t*)data; size_t *pi = data;
code_num (st->sparse_map[i].offset, keyword, xhdr); code_num (st->sparse_map[*pi].offset, keyword, xhdr);
} }
static void static void
@@ -1088,15 +1088,21 @@ sparse_offset_decoder (struct tar_stat_info *st, char const *arg)
{ {
uintmax_t u; uintmax_t u;
if (decode_num (&u, arg, TYPE_MAXIMUM (off_t), "GNU.sparse.offset")) if (decode_num (&u, arg, TYPE_MAXIMUM (off_t), "GNU.sparse.offset"))
st->sparse_map[st->sparse_map_avail].offset = u; {
if (st->sparse_map_avail < st->sparse_map_size)
st->sparse_map[st->sparse_map_avail].offset = u;
else
ERROR ((0, 0, _("Malformed extended header: excess %s=%s"),
"GNU.sparse.offset", arg));
}
} }
static void static void
sparse_numbytes_coder (struct tar_stat_info const *st, char const *keyword, sparse_numbytes_coder (struct tar_stat_info const *st, char const *keyword,
struct xheader *xhdr, void *data) struct xheader *xhdr, void *data)
{ {
size_t i = *(size_t*)data; size_t *pi = data;
code_num (st->sparse_map[i].numbytes, keyword, xhdr); code_num (st->sparse_map[*pi].numbytes, keyword, xhdr);
} }
static void static void
@@ -1105,11 +1111,11 @@ sparse_numbytes_decoder (struct tar_stat_info *st, char const *arg)
uintmax_t u; uintmax_t u;
if (decode_num (&u, arg, SIZE_MAX, "GNU.sparse.numbytes")) if (decode_num (&u, arg, SIZE_MAX, "GNU.sparse.numbytes"))
{ {
if (st->sparse_map_avail == st->sparse_map_size) if (st->sparse_map_avail < st->sparse_map_size)
st->sparse_map = x2nrealloc (st->sparse_map, st->sparse_map[st->sparse_map_avail++].numbytes = u;
&st->sparse_map_size, else
sizeof st->sparse_map[0]); ERROR ((0, 0, _("Malformed extended header: excess %s=%s"),
st->sparse_map[st->sparse_map_avail++].numbytes = u; "GNU.sparse.numbytes", arg));
} }
} }