{"thread":{"id":"4020","subject":"[PATCH] cache-tree: replace a sscanf() by two strtol() calls","startedAt":"2006-05-02T01:31:02Z","lastAt":"2006-05-02T23:26:18Z","messageCount":2,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"19343","messageId":"Pine.LNX.4.63.0605020327400.31493@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4020","inReplyTo":null,"subject":"[PATCH] cache-tree: replace a sscanf() by two strtol() calls","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-05-02T01:31:02Z","receivedAt":"2006-05-02T01:31:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nOn one of my systems, sscanf() first calls strlen() on the buffer. But\nthis buffer is not terminated by NUL. So git crashed.\n\nstrtol() does not share that problem, as it stops reading after the\nfirst non-digit.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\n---\n\n\tMaybe, a better solution would be to store the integers in \n\tbinary form. But I am not familiar with that part of git, and\n\tfurther, it would break setups which already have an index\n\twith cache-tree information.\n\n cache-tree.c |   11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\n4325bb506d03a0e30a5d4dd197601a53f0375df9\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 28b78f8..bd7c1aa 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -439,6 +439,7 @@ void *cache_tree_write(struct cache_tree\n static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)\n {\n \tconst char *buf = *buffer;\n+\tchar *endptr;\n \tunsigned long size = *size_p;\n \tstruct cache_tree *it;\n \tint i, subtree_nr;\n@@ -453,8 +454,16 @@ static struct cache_tree *read_one(const\n \t\tgoto free_return;\n \tbuf++; size--;\n \tit = cache_tree();\n-\tif (sscanf(buf, \"%d %d\\n\", &it->entry_count, &subtree_nr) != 2)\n+\tit->entry_count = strtol(buf, &endptr, 10);\n+\tif (buf == endptr)\n \t\tgoto free_return;\n+\tsize -= (endptr - buf);\n+\tbuf = endptr + 1;\n+\tsubtree_nr = strtol(buf, &endptr, 10);\n+\tif (buf == endptr)\n+\t\tgoto free_return;\n+\tsize -= (endptr - buf);\n+\tbuf = endptr + 1;\n \twhile (size && *buf && *buf != '\\n') {\n \t\tsize--;\n \t\tbuf++;\n-- \n1.3.1.g5d53-dirty\n"},{"id":"19386","messageId":"7v1wvcknz9.fsf@assigned-by-dhcp.cox.net","threadId":"4020","inReplyTo":"Pine.LNX.4.63.0605020327400.31493@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] cache-tree: replace a sscanf() by two strtol() calls","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-02T23:26:18Z","receivedAt":"2006-05-02T23:26:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On one of my systems, sscanf() first calls strlen() on the buffer. But\n> this buffer is not terminated by NUL. So git crashed.\n\nInteresting.\n\n\"git grep sscanf next -- '*.c'\" reports handful sscanf() there.\nMost of them scan argv[], which are NUL terminated, so they\nshould be OK.  The rest in convert-objects.c, tree-walk.c, and\ntree.c all scan the mode bits in tree objects, which will later\nhave the pathname component terminated with NUL, so that would\nalso be OK.\n\nBut I think your patch is wrong, and makes it ignore cache-tree\nstructure; I suspect you have two off-by-one errors and are\nmaking buf and size out of sync.\n\n> \tMaybe, a better solution would be to store the integers in \n> \tbinary form. But I am not familiar with that part of git, and\n> \tfurther, it would break setups which already have an index\n> \twith cache-tree information.\n\nIn theory, it is stashed in the extension section of index so we\ncould define a new extension type, say \"TRE2\" and store the\ninformation in the new format.  But this is purely a cache used\nas optimiation, so we could just say, \"make sure to save local\nmodifications before doing an update, then run 'rm .git/index &&\ngit-read-tree HEAD' please\".\n\nI've applied a fixed up one, but I am actually thinking about\nripping out the whole cache-tree thing and redoing it all in the\nindex.\n \nCurrently the index stores set of blobs after flattening the\nhierarchical tree structure, losing the original \"tree\"\ninformation.  We could instead store something that looks like\n\"ls-tree -t -r\" output (plus the toplevel tree information which\n\"ls-tree -t -r\" does not give you).  Just like an update-index\non the path t/t0000-basic.sh invalidates the cache-tree entry\nfor \"\" and \"t/\", we could either invalidate or recompute (I am\ninclined to do the former to make things lazy) these \"tree\"\nentries in the index.  This would be more direct way to store\nwhat I am storing in the cache-tree.\n\nKeeping the object names of unchanged subtrees available will\nallow us to walk the index and a tree (or two or more trees) in\nparallel in various applications.  \"diff-index --cached\" and\n\"read-tree -m\" extract one entry from tree and index for each\nblob, but when we have an up-to-date information for a subtree\nin the index, and when that subtree matches the corresponding\nsubtree in the tree object diff-index or read-tree is reading,\nthe application can short-cut without reading anything in the\nsubtree.\n"}]}