[PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag

[PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag

[Björn Engelmann]

Signed-off-by: Björn Engelmann <BjEngelmann@gmx.de>


---

5653af26c2cdb31e1f5646b8e563cc8fa27a8d43
 cache.h     |    1 +
 mktag.c     |   29 ++++++++++++++++-------------
 sha1_file.c |   46 ++++++++++++++++++++++++++++++++++++----------
 3 files changed, 53 insertions(+), 23 deletions(-)

5653af26c2cdb31e1f5646b8e563cc8fa27a8d43
diff --git a/cache.h b/cache.h
index 4b7a439..19e90eb 100644
--- a/cache.h
+++ b/cache.h
@@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en
 extern int ce_modified(struct cache_entry *ce, struct stat *st, int);
 extern int ce_path_match(const struct cache_entry *ce, const char
**pathspec);
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int
write_object, const char *type);
+extern int read_pipe(int fd, char** return_buf, unsigned long*
return_size);
 extern int index_pipe(unsigned char *sha1, int fd, const char *type,
int write_object);
 extern int index_path(unsigned char *sha1, const char *path, struct
stat *st, int write_object);
 extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
diff --git a/mktag.c b/mktag.c
index 2328878..79c466c 100644
--- a/mktag.c
+++ b/mktag.c
@@ -45,7 +45,7 @@ static int verify_tag(char *buffer, unsi
     unsigned char sha1[20];
     const char *object, *type_line, *tag_line, *tagger_line;
 
-    if (size < 64 || size > MAXSIZE-1)
+    if (size < 64)
         return -1;
     buffer[size] = 0;
 
@@ -105,8 +105,8 @@ static int verify_tag(char *buffer, unsi
 
 int main(int argc, char **argv)
 {
-    unsigned long size;
-    char buffer[MAXSIZE];
+    unsigned long size = 4096;
+    char *buffer = malloc(size);
     unsigned char result_sha1[20];
 
     if (argc != 1)
@@ -114,21 +114,24 @@ int main(int argc, char **argv)
 
     setup_git_directory();
 
-    // Read the signature
-    size = 0;
-    for (;;) {
-        int ret = xread(0, buffer + size, MAXSIZE - size);
-        if (ret <= 0)
-            break;
-        size += ret;
+    if (read_pipe(0, &buffer, &size)) {
+        free(buffer);
+        die("could not read from stdin");
     }
-
+    
     // Verify it for some basic sanity: it needs to start with "object
<sha1>\ntype\ntagger "
-    if (verify_tag(buffer, size) < 0)
+    if (verify_tag(buffer, size) < 0) {
+        free(buffer);
         die("invalid tag signature file");
+    }
 
-    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
+    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {
+        free(buffer);
         die("unable to write tag file");
+    }
+        
+    free(buffer);
+    
     printf("%s\n", sha1_to_hex(result_sha1));
     return 0;
 }
diff --git a/sha1_file.c b/sha1_file.c
index 2230010..ad02255 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1645,16 +1645,24 @@ int has_sha1_file(const unsigned char *s
     return find_sha1_file(sha1, &st) ? 1 : 0;
 }
 
-int index_pipe(unsigned char *sha1, int fd, const char *type, int
write_object)
+/*
+ * reads from fd as long as possible into a supplied buffer of size bytes.
+ * If neccessary the buffer's size is increased using realloc()
+ *
+ * returns 0 if anything went fine and -1 otherwise
+ *
+ * NOTE: both buf and size may change, but even when -1 is returned
+ * you still have to free() it yourself.
+ */
+int read_pipe(int fd, char** return_buf, unsigned long* return_size)
 {
-    unsigned long size = 4096;
-    char *buf = malloc(size);
-    int iret, ret;
+    char* buf = *return_buf;
+    unsigned long size = *return_size;
+    int iret;
     unsigned long off = 0;
-    unsigned char hdr[50];
-    int hdrlen;
+    
     do {
-        iret = read(fd, buf + off, size - off);
+        iret = xread(fd, buf + off, size - off);
         if (iret > 0) {
             off += iret;
             if (off == size) {
@@ -1663,16 +1671,34 @@ int index_pipe(unsigned char *sha1, int
             }
         }
     } while (iret > 0);
-    if (iret < 0) {
+    
+    *return_buf = buf;
+    *return_size = off;
+    
+    if (iret < 0)
+        return -1;
+    return 0;
+}
+
+int index_pipe(unsigned char *sha1, int fd, const char *type, int
write_object)
+{
+    unsigned long size = 4096;
+    char *buf = malloc(size);
+    int ret;
+    unsigned char hdr[50];
+    int hdrlen;
+    
+    if (read_pipe(fd, &buf, &size)) {
         free(buf);
         return -1;
     }
+    
     if (!type)
         type = blob_type;
     if (write_object)
-        ret = write_sha1_file(buf, off, type, sha1);
+        ret = write_sha1_file(buf, size, type, sha1);
     else {
-        write_sha1_file_prepare(buf, off, type, sha1, hdr, &hdrlen);
+        write_sha1_file_prepare(buf, size, type, sha1, hdr, &hdrlen);
         ret = 0;
     }
     free(buf);
-- 
1.3.3.g5045-dirty

Re: [PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> @@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en
>  extern int ce_modified(struct cache_entry *ce, struct stat *st, int);
>  extern int ce_path_match(const struct cache_entry *ce, const char
> **pathspec);
>  extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int
> write_object, const char *type);
> +extern int read_pipe(int fd, char** return_buf, unsigned long*
> return_size);
>  extern int index_pipe(unsigned char *sha1, int fd, const char *type,
> int write_object);

I smell whitespace breakage around here...

> diff --git a/mktag.c b/mktag.c
> index 2328878..79c466c 100644
> --- a/mktag.c
> +++ b/mktag.c
>...
> @@ -114,21 +114,24 @@ int main(int argc, char **argv)
>  
>      setup_git_directory();
>  
> -    // Read the signature
> -    size = 0;
> -    for (;;) {
> -        int ret = xread(0, buffer + size, MAXSIZE - size);
> -        if (ret <= 0)
> -            break;
> -        size += ret;
> +    if (read_pipe(0, &buffer, &size)) {
> +        free(buffer);
> +        die("could not read from stdin");
>      }
> -
> +    

Please do not introduce lines with trailing whitespaces.

>      // Verify it for some basic sanity: it needs to start with "object
> <sha1>\ntype\ntagger "
> -    if (verify_tag(buffer, size) < 0)
> +    if (verify_tag(buffer, size) < 0) {
> +        free(buffer);
>          die("invalid tag signature file");
> +    }

You seem to be striving for extra cleanliness, but I personally
consider it is not worth calling free() immediately before you
call die().

>  
> -    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
> +    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {
> +        free(buffer);
>          die("unable to write tag file");
> +    }
> +        
> +    free(buffer);
> +    
>      printf("%s\n", sha1_to_hex(result_sha1));
>      return 0;
>  }

A call to free() immediately before returning from main() might
look similar to the die() issue I mentioned above, but we might
extend it to do a lot more after writing the tag in the future,
so this one is very good to keep.