{"thread":{"id":"7731","subject":"[PATCH] Simplify calling of CR/LF conversion routines","startedAt":"2007-04-18T22:28:27Z","lastAt":"2007-04-22T16:16:02Z","messageCount":4,"participants":["Alex Riesen","Martin Waitz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"39836","messageId":"20070418222827.GB2477@steel.home","threadId":"7731","inReplyTo":null,"subject":"[PATCH] Simplify calling of CR/LF conversion routines","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-18T22:28:27Z","receivedAt":"2007-04-18T22:28:27Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"What is the point of having a return value if it is discarded\nafterwords anyway?\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nIt passes the tests.\n\n builtin-apply.c |   18 +++++++-----------\n cache.h         |    4 ++--\n convert.c       |   46 +++++++++++++++++++++-------------------------\n diff.c          |    4 ++--\n entry.c         |    7 ++-----\n sha1_file.c     |    7 +++----\n 6 files changed, 37 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex fd92ef7..ccd342c 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1475,8 +1475,8 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign\n \t\t}\n \t\tclose(fd);\n \t\tnsize = got;\n-\t\tnbuf = buf;\n-\t\tif (convert_to_git(path, &nbuf, &nsize)) {\n+\t\tnbuf = convert_to_git(path, buf, &nsize);\n+\t\tif (nbuf) {\n \t\t\tfree(buf);\n \t\t\t*buf_p = nbuf;\n \t\t\t*alloc_p = nsize;\n@@ -2355,9 +2355,8 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \n static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)\n {\n-\tint fd, converted;\n+\tint fd;\n \tchar *nbuf;\n-\tunsigned long nsize;\n \n \tif (has_symlinks && S_ISLNK(mode))\n \t\t/* Although buf:size is counted string, it also is NUL\n@@ -2369,13 +2368,10 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tif (fd < 0)\n \t\treturn -1;\n \n-\tnsize = size;\n-\tnbuf = (char *) buf;\n-\tconverted = convert_to_working_tree(path, &nbuf, &nsize);\n-\tif (converted) {\n+\tnbuf = convert_to_working_tree(path, buf, &size);\n+\tif (nbuf)\n \t\tbuf = nbuf;\n-\t\tsize = nsize;\n-\t}\n+\n \twhile (size) {\n \t\tint written = xwrite(fd, buf, size);\n \t\tif (written < 0)\n@@ -2387,7 +2383,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \t}\n \tif (close(fd) < 0)\n \t\tdie(\"closing file %s: %s\", path, strerror(errno));\n-\tif (converted)\n+\tif (nbuf)\n \t\tfree(nbuf);\n \treturn 0;\n }\ndiff --git a/cache.h b/cache.h\nindex 53c2341..67fd050 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -493,8 +493,8 @@ extern void trace_printf(const char *format, ...);\n extern void trace_argv_printf(const char **argv, int count, const char *format, ...);\n \n /* convert.c */\n-extern int convert_to_git(const char *path, char **bufp, unsigned long *sizep);\n-extern int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep);\n+extern char *convert_to_git(const char *path, const char *bufp, unsigned long *sizep);\n+extern char *convert_to_working_tree(const char *path, const char *bufp, unsigned long *sizep);\n \n /* match-trees.c */\n void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);\ndiff --git a/convert.c b/convert.c\nindex 898bfe3..b219c39 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -72,7 +72,7 @@ static int is_binary(unsigned long size, struct text_stat *stats)\n \treturn 0;\n }\n \n-int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n+char *convert_to_git(const char *path, const char *bufp, unsigned long *sizep)\n {\n \tchar *buffer, *nbuf;\n \tunsigned long size, nsize;\n@@ -84,18 +84,17 @@ int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n \t * stupid auto-CRLF one.\n \t */\n \tif (!auto_crlf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tsize = *sizep;\n \tif (!size)\n-\t\treturn 0;\n-\tbuffer = *bufp;\n+\t\treturn NULL;\n \n-\tgather_stats(buffer, size, &stats);\n+\tgather_stats(bufp, size, &stats);\n \n \t/* No CR? Nothing to convert, regardless. */\n \tif (!stats.cr)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/*\n \t * We're currently not going to even try to convert stuff\n@@ -103,32 +102,31 @@ int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\n \t * stuff?\n \t */\n \tif (stats.cr != stats.crlf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/*\n \t * And add some heuristics for binary vs text, of course...\n \t */\n \tif (is_binary(size, &stats))\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/*\n \t * Ok, allocate a new buffer, fill it in, and return true\n \t * to let the caller know that we switched buffers on it.\n \t */\n \tnsize = size - stats.crlf;\n-\tnbuf = xmalloc(nsize);\n-\t*bufp = nbuf;\n+\tbuffer = nbuf = xmalloc(nsize);\n \t*sizep = nsize;\n \tdo {\n-\t\tunsigned char c = *buffer++;\n+\t\tunsigned char c = *bufp++;\n \t\tif (c != '\\r')\n \t\t\t*nbuf++ = c;\n \t} while (--size);\n \n-\treturn 1;\n+\treturn buffer;\n }\n \n-int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n+char *convert_to_working_tree(const char *path, const char *bufp, unsigned long *sizep)\n {\n \tchar *buffer, *nbuf;\n \tunsigned long size, nsize;\n@@ -141,46 +139,44 @@ int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)\n \t * stupid auto-CRLF one.\n \t */\n \tif (auto_crlf <= 0)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tsize = *sizep;\n \tif (!size)\n-\t\treturn 0;\n-\tbuffer = *bufp;\n+\t\treturn NULL;\n \n-\tgather_stats(buffer, size, &stats);\n+\tgather_stats(bufp, size, &stats);\n \n \t/* No LF? Nothing to convert, regardless. */\n \tif (!stats.lf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/* Was it already in CRLF format? */\n \tif (stats.lf == stats.crlf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/* If we have any bare CR characters, we're not going to touch it */\n \tif (stats.cr != stats.crlf)\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \tif (is_binary(size, &stats))\n-\t\treturn 0;\n+\t\treturn NULL;\n \n \t/*\n \t * Ok, allocate a new buffer, fill it in, and return true\n \t * to let the caller know that we switched buffers on it.\n \t */\n \tnsize = size + stats.lf - stats.crlf;\n-\tnbuf = xmalloc(nsize);\n-\t*bufp = nbuf;\n+\tbuffer = nbuf = xmalloc(nsize);\n \t*sizep = nsize;\n \tlast = 0;\n \tdo {\n-\t\tunsigned char c = *buffer++;\n+\t\tunsigned char c = *bufp++;\n \t\tif (c == '\\n' && last != '\\r')\n \t\t\t*nbuf++ = '\\r';\n \t\t*nbuf++ = c;\n \t\tlast = c;\n \t} while (--size);\n \n-\treturn 1;\n+\treturn buffer;\n }\ndiff --git a/diff.c b/diff.c\nindex fbb79d7..ee72cc6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1461,9 +1461,9 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\t/*\n \t\t * Convert from working tree format to canonical git format\n \t\t */\n-\t\tbuf = s->data;\n \t\tsize = s->size;\n-\t\tif (convert_to_git(s->path, &buf, &size)) {\n+\t\tbuf = convert_to_git(s->path, s->data, &size);\n+\t\tif (buf) {\n \t\t\tmunmap(s->data, s->size);\n \t\t\ts->should_munmap = 0;\n \t\t\ts->data = buf;\ndiff --git a/entry.c b/entry.c\nindex d72f811..3771209 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -79,7 +79,6 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \t}\n \tswitch (ntohl(ce->ce_mode) & S_IFMT) {\n \t\tchar *buf;\n-\t\tunsigned long nsize;\n \n \tcase S_IFREG:\n \t\tif (to_tempfile) {\n@@ -96,12 +95,10 @@ static int write_entry(struct cache_entry *ce, char *path, struct checkout *stat\n \t\t/*\n \t\t * Convert from git internal format to working tree format\n \t\t */\n-\t\tbuf = new;\n-\t\tnsize = size;\n-\t\tif (convert_to_working_tree(ce->name, &buf, &nsize)) {\n+\t\tbuf = convert_to_working_tree(ce->name, new, &size);\n+\t\tif (buf) {\n \t\t\tfree(new);\n \t\t\tnew = buf;\n-\t\t\tsize = nsize;\n \t\t}\n \n \t\twrote = write_in_full(fd, new, size);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4304fe9..1978d5f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2277,10 +2277,9 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,\n \t */\n \tif ((type == OBJ_BLOB) && S_ISREG(st->st_mode)) {\n \t\tunsigned long nsize = size;\n-\t\tchar *nbuf = buf;\n-\t\tif (convert_to_git(path, &nbuf, &nsize)) {\n-\t\t\tif (size)\n-\t\t\t\tmunmap(buf, size);\n+\t\tchar *nbuf = convert_to_git(path, buf, &nsize);\n+\t\tif (nbuf) {\n+\t\t\tmunmap(buf, size);\n \t\t\tsize = nsize;\n \t\t\tbuf = nbuf;\n \t\t\tre_allocated = 1;\n-- \n1.5.1.1.876.ge36f76\n"},{"id":"39839","messageId":"20070418230334.GD2477@steel.home","threadId":"7731","inReplyTo":"20070418222827.GB2477@steel.home","subject":"Re: [PATCH] Simplify calling of CR/LF conversion routines","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-18T23:03:34Z","receivedAt":"2007-04-18T23:03:34Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Alex Riesen, Thu, Apr 19, 2007 00:28:27 +0200:\n> What is the point of having a return value if it is discarded\n> afterwords anyway?\n> \n\nIgnore this. I'll rediff against the \"next\" branch\n"},{"id":"40116","messageId":"20070422134505.GF27208@admingilde.org","threadId":"7731","inReplyTo":"20070418222827.GB2477@steel.home","subject":"Re: [PATCH] Simplify calling of CR/LF conversion routines","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-04-22T13:45:06Z","receivedAt":"2007-04-22T13:45:06Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nFor me (on glibc-2.5) the testsuite fails in t0020-crlf.sh, test 14:\n\n*** glibc detected *** git: free(): invalid next size (fast): 0x081097b8 ***\n======= Backtrace: =========\n/lib/i686/cmov/libc.so.6[0x401ed843]\n/lib/i686/cmov/libc.so.6(__libc_free+0x90)[0x401f0d00]\ngit[0x8096f66]\ngit[0x808ca97]\ngit[0x808cf52]\ngit[0x80715b2]\ngit[0x804a957]\ngit[0x804adac]\n/lib/i686/cmov/libc.so.6(__libc_start_main+0xd8)[0x4019b878]\ngit[0x804a6d1]\n======= Memory map: ========\n08048000-080ca000 r-xp 00000000 fe:03 856305     /home/martin/src/git/git\n080ca000-080cc000 rw-p 00082000 fe:03 856305     /home/martin/src/git/git\n080cc000-0812a000 rw-p 080cc000 00:00 0          [heap]\n40000000-4001c000 r-xp 00000000 fe:05 1505       /lib/ld-2.5.so\n4001c000-4001e000 rw-p 0001b000 fe:05 1505       /lib/ld-2.5.so\n4001e000-4001f000 r-xp 4001e000 00:00 0          [vdso]\n4001f000-40021000 rw-p 4001f000 00:00 0\n40021000-40022000 rw-p 00000000 fe:03 902199     /home/martin/src/git/t/trash/.git/index\n40022000-4002c000 r-xp 00000000 fe:05 2937       /lib/libgcc_s.so.1\n4002c000-4002d000 rw-p 00009000 fe:05 2937       /lib/libgcc_s.so.1\n40035000-40048000 r-xp 00000000 fe:09 98501      /usr/lib/libz.so.1.2.3\n40048000-40049000 rw-p 00012000 fe:09 98501      /usr/lib/libz.so.1.2.3\n40049000-4016d000 r-xp 00000000 fe:09 114802     /usr/lib/i686/cmov/libcrypto.so.0.9.8\n4016d000-40181000 rw-p 00123000 fe:09 114802     /usr/lib/i686/cmov/libcrypto.so.0.9.8\n40181000-40186000 rw-p 40181000 00:00 0\n40186000-402c1000 r-xp 00000000 fe:05 1983       /lib/i686/cmov/libc-2.5.so\n402c1000-402c2000 r--p 0013b000 fe:05 1983       /lib/i686/cmov/libc-2.5.so\n402c2000-402c4000 rw-p 0013c000 fe:05 1983       /lib/i686/cmov/libc-2.5.so\n402c4000-402c7000 rw-p 402c4000 00:00 0\n402c7000-402c9000 r-xp 00000000 fe:05 2194       /lib/i686/cmov/libdl-2.5.so\n402c9000-402cb000 rw-p 00001000 fe:05 2194       /lib/i686/cmov/libdl-2.5.so\n402cb000-402cc000 rw-p 402cb000 00:00 0\n40300000-40321000 rw-p 40300000 00:00 0\n40321000-40400000 ---p 40321000 00:00 0\nbff3f000-bff54000 rw-p bff3f000 00:00 0          [stack]\n* FAIL 14: .gitattributes says two and three are text\n\n\nbisecting points to ac78e548, but I can't see any obvious problem with\nthis commit.\nDo you have any idea?\n\n-- \nMartin Waitz\n"},{"id":"40123","messageId":"20070422161602.GD2431@steel.home","threadId":"7731","inReplyTo":"20070422134505.GF27208@admingilde.org","subject":"Re: [PATCH] Simplify calling of CR/LF conversion routines","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-22T16:16:02Z","receivedAt":"2007-04-22T16:16:02Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Martin Waitz, Sun, Apr 22, 2007 15:45:06 +0200:\n> hoi :)\n> \n> For me (on glibc-2.5) the testsuite fails in t0020-crlf.sh, test 14:\n> \n> *** glibc detected *** git: free(): invalid next size (fast): 0x081097b8 ***\n\nYep, already fixed. The patches attached (I forgot which one was the\nfix for this particular problem).\n\n\n\n>From 001f0b86bc4142743eea242649fb188807e03c16 Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen <raa.lkml@gmail.com>\nDate: Sun, 22 Apr 2007 16:03:49 +0200\nSubject: [PATCH] Fix crash in t0020 (crlf conversion)\n\nReallocated wrong size.\nNoticed on Ubuntu 7.04 probably because it has some malloc diagnostics in libc:\n\"git-read-tree --reset -u HEAD\" aborted in the test. Valgrind sped up the\ndebugging greatly: took me 10 minutes.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n attr.c |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 285e689..a071254 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -300,7 +300,8 @@ static struct attr_stack *read_attr_from_array(const char **list)\n \t\ta = parse_attr_line(line, \"[builtin]\", ++lineno, 1);\n \t\tif (!a)\n \t\t\tcontinue;\n-\t\tres->attrs = xrealloc(res->attrs, res->num_matches + 1);\n+\t\tres->attrs = xrealloc(res->attrs,\n+\t\t\tsizeof(struct match_attr *) * (res->num_matches + 1));\n \t\tres->attrs[res->num_matches++] = a;\n \t}\n \treturn res;\n@@ -324,7 +325,8 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)\n \t\ta = parse_attr_line(buf, path, ++lineno, macro_ok);\n \t\tif (!a)\n \t\t\tcontinue;\n-\t\tres->attrs = xrealloc(res->attrs, res->num_matches + 1);\n+\t\tres->attrs = xrealloc(res->attrs,\n+\t\t\tsizeof(struct match_attr *) * (res->num_matches + 1));\n \t\tres->attrs[res->num_matches++] = a;\n \t}\n \tfclose(fp);\n-- \n1.5.1.1.946.gdb75a\n\n\n\n>From e306988bdabf5dec30c945167bfaa5f5f2c16835 Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen <raa.lkml@gmail.com>\nDate: Sun, 22 Apr 2007 16:07:19 +0200\nSubject: [PATCH] Fix a typo in crlf conversion code\n\nAlso, noticed by valgrind: the code caused a read out-of-bounds.\nSome comments updated as well (they still reflected old calling\nconventions).\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n convert.c |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 37239ac..ad106ef 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -115,8 +115,8 @@ static char *crlf_to_git(const char *path, const char *src, unsigned long *sizep\n \t}\n \n \t/*\n-\t * Ok, allocate a new buffer, fill it in, and return true\n-\t * to let the caller know that we switched buffers on it.\n+\t * Ok, allocate a new buffer, fill it in, and return it\n+\t * to let the caller know that we switched buffers.\n \t */\n \tnsize = size - stats.crlf;\n \tbuffer = xmalloc(nsize);\n@@ -137,7 +137,7 @@ static char *crlf_to_git(const char *path, const char *src, unsigned long *sizep\n \t} else {\n \t\tdo {\n \t\t\tunsigned char c = *src++;\n-\t\t\tif (! (c == '\\r' && (1 < size && *buffer == '\\n')))\n+\t\t\tif (! (c == '\\r' && (1 < size && *src == '\\n')))\n \t\t\t\t*dst++ = c;\n \t\t} while (--size);\n \t}\n@@ -180,8 +180,8 @@ static char *crlf_to_worktree(const char *path, const char *src, unsigned long *\n \t}\n \n \t/*\n-\t * Ok, allocate a new buffer, fill it in, and return true\n-\t * to let the caller know that we switched buffers on it.\n+\t * Ok, allocate a new buffer, fill it in, and return it\n+\t * to let the caller know that we switched buffers.\n \t */\n \tnsize = size + stats.lf - stats.crlf;\n \tbuffer = xmalloc(nsize);\n-- \n1.5.1.1.946.gdb75a\n\n"}]}