{"thread":{"id":"4244","subject":"[PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag","startedAt":"2006-05-22T14:48:58Z","lastAt":"2006-05-22T19:19:33Z","messageCount":2,"participants":["Björn Engelmann","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"20452","messageId":"4471CF5A.702@gmx.de","threadId":"4244","inReplyTo":null,"subject":"[PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag","fromName":"Björn Engelmann","fromEmail":"bjengelmann@gmx.de","sentAt":"2006-05-22T14:48:58Z","receivedAt":"2006-05-22T14:48:58Z","isPatch":true,"sender":{"key":"bjengelmann@gmx.de","avatar":null},"body":"\nSigned-off-by: Björn Engelmann <BjEngelmann@gmx.de>\n\n\n---\n\n5653af26c2cdb31e1f5646b8e563cc8fa27a8d43\n cache.h     |    1 +\n mktag.c     |   29 ++++++++++++++++-------------\n sha1_file.c |   46 ++++++++++++++++++++++++++++++++++++----------\n 3 files changed, 53 insertions(+), 23 deletions(-)\n\n5653af26c2cdb31e1f5646b8e563cc8fa27a8d43\ndiff --git a/cache.h b/cache.h\nindex 4b7a439..19e90eb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en\n extern int ce_modified(struct cache_entry *ce, struct stat *st, int);\n extern int ce_path_match(const struct cache_entry *ce, const char\n**pathspec);\n extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int\nwrite_object, const char *type);\n+extern int read_pipe(int fd, char** return_buf, unsigned long*\nreturn_size);\n extern int index_pipe(unsigned char *sha1, int fd, const char *type,\nint write_object);\n extern int index_path(unsigned char *sha1, const char *path, struct\nstat *st, int write_object);\n extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);\ndiff --git a/mktag.c b/mktag.c\nindex 2328878..79c466c 100644\n--- a/mktag.c\n+++ b/mktag.c\n@@ -45,7 +45,7 @@ static int verify_tag(char *buffer, unsi\n     unsigned char sha1[20];\n     const char *object, *type_line, *tag_line, *tagger_line;\n \n-    if (size < 64 || size > MAXSIZE-1)\n+    if (size < 64)\n         return -1;\n     buffer[size] = 0;\n \n@@ -105,8 +105,8 @@ static int verify_tag(char *buffer, unsi\n \n int main(int argc, char **argv)\n {\n-    unsigned long size;\n-    char buffer[MAXSIZE];\n+    unsigned long size = 4096;\n+    char *buffer = malloc(size);\n     unsigned char result_sha1[20];\n \n     if (argc != 1)\n@@ -114,21 +114,24 @@ int main(int argc, char **argv)\n \n     setup_git_directory();\n \n-    // Read the signature\n-    size = 0;\n-    for (;;) {\n-        int ret = xread(0, buffer + size, MAXSIZE - size);\n-        if (ret <= 0)\n-            break;\n-        size += ret;\n+    if (read_pipe(0, &buffer, &size)) {\n+        free(buffer);\n+        die(\"could not read from stdin\");\n     }\n-\n+    \n     // Verify it for some basic sanity: it needs to start with \"object\n<sha1>\\ntype\\ntagger \"\n-    if (verify_tag(buffer, size) < 0)\n+    if (verify_tag(buffer, size) < 0) {\n+        free(buffer);\n         die(\"invalid tag signature file\");\n+    }\n \n-    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)\n+    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {\n+        free(buffer);\n         die(\"unable to write tag file\");\n+    }\n+        \n+    free(buffer);\n+    \n     printf(\"%s\\n\", sha1_to_hex(result_sha1));\n     return 0;\n }\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 2230010..ad02255 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1645,16 +1645,24 @@ int has_sha1_file(const unsigned char *s\n     return find_sha1_file(sha1, &st) ? 1 : 0;\n }\n \n-int index_pipe(unsigned char *sha1, int fd, const char *type, int\nwrite_object)\n+/*\n+ * reads from fd as long as possible into a supplied buffer of size bytes.\n+ * If neccessary the buffer's size is increased using realloc()\n+ *\n+ * returns 0 if anything went fine and -1 otherwise\n+ *\n+ * NOTE: both buf and size may change, but even when -1 is returned\n+ * you still have to free() it yourself.\n+ */\n+int read_pipe(int fd, char** return_buf, unsigned long* return_size)\n {\n-    unsigned long size = 4096;\n-    char *buf = malloc(size);\n-    int iret, ret;\n+    char* buf = *return_buf;\n+    unsigned long size = *return_size;\n+    int iret;\n     unsigned long off = 0;\n-    unsigned char hdr[50];\n-    int hdrlen;\n+    \n     do {\n-        iret = read(fd, buf + off, size - off);\n+        iret = xread(fd, buf + off, size - off);\n         if (iret > 0) {\n             off += iret;\n             if (off == size) {\n@@ -1663,16 +1671,34 @@ int index_pipe(unsigned char *sha1, int\n             }\n         }\n     } while (iret > 0);\n-    if (iret < 0) {\n+    \n+    *return_buf = buf;\n+    *return_size = off;\n+    \n+    if (iret < 0)\n+        return -1;\n+    return 0;\n+}\n+\n+int index_pipe(unsigned char *sha1, int fd, const char *type, int\nwrite_object)\n+{\n+    unsigned long size = 4096;\n+    char *buf = malloc(size);\n+    int ret;\n+    unsigned char hdr[50];\n+    int hdrlen;\n+    \n+    if (read_pipe(fd, &buf, &size)) {\n         free(buf);\n         return -1;\n     }\n+    \n     if (!type)\n         type = blob_type;\n     if (write_object)\n-        ret = write_sha1_file(buf, off, type, sha1);\n+        ret = write_sha1_file(buf, size, type, sha1);\n     else {\n-        write_sha1_file_prepare(buf, off, type, sha1, hdr, &hdrlen);\n+        write_sha1_file_prepare(buf, size, type, sha1, hdr, &hdrlen);\n         ret = 0;\n     }\n     free(buf);\n-- \n1.3.3.g5045-dirty\n"},{"id":"20465","messageId":"7v1wulc1fe.fsf@assigned-by-dhcp.cox.net","threadId":"4244","inReplyTo":"4471CF5A.702@gmx.de","subject":"Re: [PATCH 1/2] removes the artificial restriction tagsize < 8kb from git-mktag","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-22T19:19:33Z","receivedAt":"2006-05-22T19:19:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Björn Engelmann <BjEngelmann@gmx.de> writes:\n\n> @@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en\n>  extern int ce_modified(struct cache_entry *ce, struct stat *st, int);\n>  extern int ce_path_match(const struct cache_entry *ce, const char\n> **pathspec);\n>  extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int\n> write_object, const char *type);\n> +extern int read_pipe(int fd, char** return_buf, unsigned long*\n> return_size);\n>  extern int index_pipe(unsigned char *sha1, int fd, const char *type,\n> int write_object);\n\nI smell whitespace breakage around here...\n\n> diff --git a/mktag.c b/mktag.c\n> index 2328878..79c466c 100644\n> --- a/mktag.c\n> +++ b/mktag.c\n>...\n> @@ -114,21 +114,24 @@ int main(int argc, char **argv)\n>  \n>      setup_git_directory();\n>  \n> -    // Read the signature\n> -    size = 0;\n> -    for (;;) {\n> -        int ret = xread(0, buffer + size, MAXSIZE - size);\n> -        if (ret <= 0)\n> -            break;\n> -        size += ret;\n> +    if (read_pipe(0, &buffer, &size)) {\n> +        free(buffer);\n> +        die(\"could not read from stdin\");\n>      }\n> -\n> +    \n\nPlease do not introduce lines with trailing whitespaces.\n\n>      // Verify it for some basic sanity: it needs to start with \"object\n> <sha1>\\ntype\\ntagger \"\n> -    if (verify_tag(buffer, size) < 0)\n> +    if (verify_tag(buffer, size) < 0) {\n> +        free(buffer);\n>          die(\"invalid tag signature file\");\n> +    }\n\nYou seem to be striving for extra cleanliness, but I personally\nconsider it is not worth calling free() immediately before you\ncall die().\n\n>  \n> -    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)\n> +    if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0) {\n> +        free(buffer);\n>          die(\"unable to write tag file\");\n> +    }\n> +        \n> +    free(buffer);\n> +    \n>      printf(\"%s\\n\", sha1_to_hex(result_sha1));\n>      return 0;\n>  }\n\nA call to free() immediately before returning from main() might\nlook similar to the die() issue I mentioned above, but we might\nextend it to do a lot more after writing the tag in the future,\nso this one is very good to keep.\n"}]}