[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-tar] [PATCH] improved error messages when reading snapshot file
From: |
Paul Eggert |
Subject: |
Re: [Bug-tar] [PATCH] improved error messages when reading snapshot file |
Date: |
Sun, 27 Jan 2013 08:47:37 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 |
Thanks for that patch. It inspired me to install the
following change to GNU tar. This is not the same patch
but it uses the same ideas. I refactored the code, which
had some unnecessary duplication that your patch exposed.
You can see the installed patch at:
http://git.savannah.gnu.org/cgit/tar.git/commit/?id=9cf743abf8667ae07077ceb32fad9a94268a5a93
>From 2cd41f8a12d837dba8f1c83bd5758fd046cb829c Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Sun, 27 Jan 2013 08:41:32 -0800
Subject: [PATCH] tar: improve quality of diagnostics with incrementals
Inspired by a prototype by Nathan Stratton Treadway in
<http://lists.gnu.org/archive/html/bug-tar/2013-01/msg00000.html>.
* src/incremen.c (read_num): Rewrite by merging read_negative_num
and read_unsigned_num. Use strtosysint rather than rolling this
stuff ourselves. Change return type to bool. All uses changed.
(read_negative_num, read_unsigned_num): Remove.
(read_num, read_timespec, read_incr_db_2): Improve quality of
diagnostics, e.g., by supplying byte offset of error.
---
src/incremen.c | 171 ++++++++++++++++++++++++++-------------------------------
1 file changed, 78 insertions(+), 93 deletions(-)
diff --git a/src/incremen.c b/src/incremen.c
index 7130bc2..1d20c44 100644
--- a/src/incremen.c
+++ b/src/incremen.c
@@ -1079,106 +1079,84 @@ read_obstack (FILE *fp, struct obstack *stk, size_t
*pcount)
return c;
}
-/* Read from file FP a nul-terminated string and convert it to
- intmax_t. Return the resulting value in PVAL. Assume '-' has
- already been read.
+/* Read from file FP a null-terminated string and convert it to an
+ integer. FIELDNAME is the intended use of the integer, useful for
+ diagnostics. MIN_VAL and MAX_VAL are its minimum and maximum
+ permissible values; MIN_VAL must be nonpositive and MAX_VAL positive.
+ Store into *PVAL the resulting value, converted to intmax_t.
Throw a fatal error if the string cannot be converted or if the
- converted value is less than MIN_VAL. */
+ converted value is out of range.
-static void
-read_negative_num (FILE *fp, intmax_t min_val, intmax_t *pval)
+ Return true if successful, false if end of file. */
+
+static bool
+read_num (FILE *fp, char const *fieldname,
+ intmax_t min_val, uintmax_t max_val, intmax_t *pval)
{
- int c;
- size_t i;
+ int i;
char buf[INT_BUFSIZE_BOUND (intmax_t)];
- char *ep;
- buf[0] = '-';
+ char offbuf[INT_BUFSIZE_BOUND (off_t)];
+ char minbuf[INT_BUFSIZE_BOUND (intmax_t)];
+ char maxbuf[INT_BUFSIZE_BOUND (intmax_t)];
+ int conversion_errno;
+ int c = getc (fp);
+ bool negative = c == '-';
- for (i = 1; ISDIGIT (c = getc (fp)); i++)
+ for (i = 0; (i == 0 && negative) || ISDIGIT (c); i++)
{
- if (i == sizeof buf - 1)
- FATAL_ERROR ((0, 0, _("Field too long while reading snapshot file")));
buf[i] = c;
- }
-
- if (c < 0)
- {
- if (ferror (fp))
- FATAL_ERROR ((0, errno, _("Read error in snapshot file")));
- else
- FATAL_ERROR ((0, 0, _("Unexpected EOF in snapshot file")));
- }
-
- buf[i] = 0;
- errno = 0;
- *pval = strtoimax (buf, &ep, 10);
- if (c || errno || *pval < min_val)
- FATAL_ERROR ((0, errno, _("Unexpected field value in snapshot file")));
-}
-
-/* Read from file FP a nul-terminated string and convert it to
- uintmax_t. Return an intmax_t representation of the resulting
- value in PVAL. Assume C has already been read.
-
- Throw a fatal error if the string cannot be converted or if the
- converted value exceeds MAX_VAL.
-
- Return the last character read or EOF on end of file. */
-
-static int
-read_unsigned_num (int c, FILE *fp, uintmax_t max_val, intmax_t *pval)
-{
- size_t i;
- uintmax_t u;
- char buf[UINTMAX_STRSIZE_BOUND], *ep;
-
- for (i = 0; ISDIGIT (c); i++)
- {
if (i == sizeof buf - 1)
- FATAL_ERROR ((0, 0, _("Field too long while reading snapshot file")));
- buf[i] = c;
+ FATAL_ERROR ((0, 0,
+ _("%s: byte %s: %s %.*s... too long"),
+ quotearg_colon (listed_incremental_option),
+ offtostr (ftello (fp), offbuf),
+ fieldname, i + 1, buf));
c = getc (fp);
}
+ buf[i] = 0;
+
if (c < 0)
{
if (ferror (fp))
- FATAL_ERROR ((0, errno, _("Read error in snapshot file")));
- else if (i == 0)
- return c;
- else
- FATAL_ERROR ((0, 0, _("Unexpected EOF in snapshot file")));
+ read_fatal (listed_incremental_option);
+ if (i != 0)
+ FATAL_ERROR ((0, 0, "%s: %s",
+ quotearg_colon (listed_incremental_option),
+ _("Unexpected EOF in snapshot file")));
+ return false;
}
- buf[i] = 0;
- errno = 0;
- u = strtoumax (buf, &ep, 10);
- if (c || errno || max_val < u)
- FATAL_ERROR ((0, errno, _("Unexpected field value in snapshot file")));
- *pval = represent_uintmax (u);
- return c;
-}
-
-/* Read from file FP a nul-terminated string and convert it to
- an integer in the range MIN_VAL..MAXVAL. Return the resulting
- value, converted to intmax_t, in PVAL. MINVAL must be nonpositive.
-
- Throw a fatal error if the string cannot be converted or if the
- converted value is out of range.
+ if (c)
+ FATAL_ERROR ((0, 0,
+ _("%s: byte %s: %s %s followed by invalid byte 0x%02x"),
+ quotearg_colon (listed_incremental_option),
+ offtostr (ftello (fp), offbuf),
+ fieldname, buf, c));
- Return the last character read or EOF on end of file. */
+ *pval = strtosysint (buf, NULL, min_val, max_val);
+ conversion_errno = errno;
-static int
-read_num (FILE *fp, intmax_t min_val, uintmax_t max_val, intmax_t *pval)
-{
- int c = getc (fp);
- if (c == '-')
+ switch (conversion_errno)
{
- read_negative_num (fp, min_val, pval);
- return 0;
+ case ERANGE:
+ FATAL_ERROR ((0, conversion_errno,
+ _("%s: byte %s: (valid range %s..%s)\n\t%s %s"),
+ quotearg_colon (listed_incremental_option),
+ offtostr (ftello (fp), offbuf),
+ imaxtostr (min_val, minbuf),
+ umaxtostr (max_val, maxbuf), fieldname, buf));
+ default:
+ FATAL_ERROR ((0, conversion_errno,
+ _("%s: byte %s: %s %s"),
+ quotearg_colon (listed_incremental_option),
+ offtostr (ftello (fp), offbuf), fieldname, buf));
+ case 0:
+ break;
}
- return read_unsigned_num (c, fp, max_val, pval);
+
+ return true;
}
/* Read from FP two NUL-terminated strings representing a struct
@@ -1189,15 +1167,20 @@ read_num (FILE *fp, intmax_t min_val, uintmax_t
max_val, intmax_t *pval)
static void
read_timespec (FILE *fp, struct timespec *pval)
{
- intmax_t i;
- int c = read_num (fp, TYPE_MINIMUM (time_t), TYPE_MAXIMUM (time_t), &i);
- pval->tv_sec = i;
+ intmax_t s, ns;
- if (c || read_num (fp, 0, BILLION - 1, &i))
- FATAL_ERROR ((0, 0, "%s: %s",
- quotearg_colon (listed_incremental_option),
- _("Unexpected EOF in snapshot file")));
- pval->tv_nsec = i;
+ if (read_num (fp, "sec", TYPE_MINIMUM (time_t), TYPE_MAXIMUM (time_t), &s)
+ && read_num (fp, "nsec", 0, BILLION - 1, &ns))
+ {
+ pval->tv_sec = s;
+ pval->tv_nsec = ns;
+ }
+ else
+ {
+ FATAL_ERROR ((0, 0, "%s: %s",
+ quotearg_colon (listed_incremental_option),
+ _("Unexpected EOF in snapshot file")));
+ }
}
/* Read incremental snapshot format 2 */
@@ -1205,6 +1188,7 @@ static void
read_incr_db_2 (void)
{
struct obstack stk;
+ char offbuf[INT_BUFSIZE_BOUND (off_t)];
obstack_init (&stk);
@@ -1221,20 +1205,20 @@ read_incr_db_2 (void)
char *content;
size_t s;
- if (read_num (listed_incremental_stream, 0, 1, &i))
+ if (! read_num (listed_incremental_stream, "nfs", 0, 1, &i))
return; /* Normal return */
nfs = i;
read_timespec (listed_incremental_stream, &mtime);
- if (read_num (listed_incremental_stream,
- TYPE_MINIMUM (dev_t), TYPE_MAXIMUM (dev_t), &i))
+ if (! read_num (listed_incremental_stream, "dev",
+ TYPE_MINIMUM (dev_t), TYPE_MAXIMUM (dev_t), &i))
break;
dev = i;
- if (read_num (listed_incremental_stream,
- TYPE_MINIMUM (ino_t), TYPE_MAXIMUM (ino_t), &i))
+ if (! read_num (listed_incremental_stream, "ino",
+ TYPE_MINIMUM (ino_t), TYPE_MAXIMUM (ino_t), &i))
break;
ino = i;
@@ -1246,8 +1230,9 @@ read_incr_db_2 (void)
while (read_obstack (listed_incremental_stream, &stk, &s) == 0 && s > 1)
;
if (getc (listed_incremental_stream) != 0)
- FATAL_ERROR ((0, 0, "%s: %s",
+ FATAL_ERROR ((0, 0, _("%s: byte %s: %s"),
quotearg_colon (listed_incremental_option),
+ offtostr (ftello (listed_incremental_stream), offbuf),
_("Missing record terminator")));
content = obstack_finish (&stk);
--
1.7.11.7