Comparing the result of read_in_full() using less-than is
potentially dangerous, as discussed in 561598cfcf
(read_pack_header: handle signed/unsigned comparison in read
result, 2017-09-13).

The instance in get-tar-commit-id is OK, because the
HEADERSIZE macro expands to a signed integer. But if it were
switched to an unsigned type like:

  size_t HEADERSIZE = ...;

this would be a bug. Let's use the more robust "!="
construct.

We can also drop the useless "n" variable while we're at it.

Suggested-by: Jonathan Nieder <jrnie...@gmail.com>
Signed-off-by: Jeff King <p...@peff.net>
---
 builtin/get-tar-commit-id.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index 6d9a79f9b3..259ad40339 100644
--- a/builtin/get-tar-commit-id.c
+++ b/builtin/get-tar-commit-id.c
@@ -20,14 +20,12 @@ int cmd_get_tar_commit_id(int argc, const char **argv, 
const char *prefix)
        struct ustar_header *header = (struct ustar_header *)buffer;
        char *content = buffer + RECORDSIZE;
        const char *comment;
-       ssize_t n;
 
        if (argc != 1)
                usage(builtin_get_tar_commit_id_usage);
 
-       n = read_in_full(0, buffer, HEADERSIZE);
-       if (n < HEADERSIZE)
-               die("git get-tar-commit-id: read error");
+       if (read_in_full(0, buffer, HEADERSIZE) != HEADERSIZE)
+               die_errno("git get-tar-commit-id: read error");
        if (header->typeflag[0] != 'g')
                return 1;
        if (!skip_prefix(content, "52 comment=", &comment))
-- 
2.14.1.1148.ga2561536a1

Reply via email to