Michael Forney wrote:
> ---
> I wouldn't bother with the Error enum, since after you assign the errnum, you
> immediately look it up in the error table. Instead, you can just set the
> reason
> string directly. I also think it's worth differentiating between an invalid
> and
> incorrect checksum.
Your patch is indeed better than mine, I must admit. I tend to avoid
assigning pointers to static strings like this, but I really don't have
any good reason to do so.
Will you submit the patch yourself?
> Here is the patch I mentioned in my previous reply.
>
> tar.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/tar.c b/tar.c
> index 2272303..44353d7 100644
> --- a/tar.c
> +++ b/tar.c
> @@ -408,30 +408,38 @@ sanitize(struct header *h)
> static void
> chktar(struct header *h)
> {
> - char tmp[8], *err;
> - char *p = (char *)h;
> + char tmp[8], *err, *p = (char *)h;
> + const char *reason;
> long s1, s2, i;
>
> - if (h->prefix[0] == '\0' && h->name[0] == '\0')
> + if (h->prefix[0] == '\0' && h->name[0] == '\0') {
> + reason = "empty filename";
> goto bad;
> - if (h->magic[0] && strncmp("ustar", h->magic, 5))
> + }
> + if (h->magic[0] && strncmp("ustar", h->magic, 5)) {
> + reason = "not ustar format";
> goto bad;
> + }
> memcpy(tmp, h->chksum, sizeof(tmp));
> for (i = 0; i < sizeof(tmp); i++)
> if (tmp[i] == ' ')
> tmp[i] = '\0';
> s1 = strtol(tmp, &err, 8);
> - if (s1 < 0 || *err != '\0')
> + if (s1 < 0 || *err != '\0') {
> + reason = "invalid checksum";
> goto bad;
> + }
> memset(h->chksum, ' ', sizeof(h->chksum));
> for (i = 0, s2 = 0; i < sizeof(*h); i++)
> s2 += (unsigned char)p[i];
> - if (s1 != s2)
> + if (s1 != s2) {
> + reason = "incorrect checksum";
> goto bad;
> + }
> memcpy(h->chksum, tmp, sizeof(h->chksum));
> return;
> bad:
> - eprintf("malformed tar archive\n");
> + eprintf("malformed tar archive: %s\n", reason);
> }
>
> static void
> --
> 2.11.0
