{"thread":{"id":"7590","subject":"sscanf/strtoul: parse integers robustly","startedAt":"2007-04-09T23:01:44Z","lastAt":"2007-04-19T02:26:41Z","messageCount":5,"participants":["Jim Meyering","Junio C Hamano","Andy Whitcroft"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"38976","messageId":"871witxicn.fsf@rho.meyering.net","threadId":"7590","inReplyTo":null,"subject":"sscanf/strtoul: parse integers robustly","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2007-04-09T23:01:44Z","receivedAt":"2007-04-09T23:01:44Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"* builtin-grep.c (strtoul_ui): Move function definition from here, to...\n* git-compat-util.h (strtoul_ui): ...here, with an added \"base\" parameter.\n* builtin-grep.c (cmd_grep): Update use of strtoul_ui to include base, \"10\".\n* builtin-update-index.c (read_index_info): Diagnose an invalid mode integer\nthat is out of range or merely larger than INT_MAX.\n(cmd_update_index): Use strtoul_ui, not sscanf.\n* convert-objects.c (write_subdirectory): Likewise.\n\nSigned-off-by: Jim Meyering <jim@meyering.net>\n---\n builtin-grep.c         |   15 +--------------\n builtin-update-index.c |   10 +++++++---\n convert-objects.c      |    2 +-\n git-compat-util.h      |   13 +++++++++++++\n 4 files changed, 22 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 981f3d4..e13cb31 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -434,19 +434,6 @@ static const char emsg_missing_context_len[] =\n static const char emsg_missing_argument[] =\n \"option requires an argument -%s\";\n \n-static int strtoul_ui(char const *s, unsigned int *result)\n-{\n-\tunsigned long ul;\n-\tchar *p;\n-\n-\terrno = 0;\n-\tul = strtoul(s, &p, 10);\n-\tif (errno || *p || p == s || (unsigned int) ul != ul)\n-\t\treturn -1;\n-\t*result = ul;\n-\treturn 0;\n-}\n-\n int cmd_grep(int argc, const char **argv, const char *prefix)\n {\n \tint hit = 0;\n@@ -569,7 +556,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t\tscan = arg + 1;\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\tif (strtoul_ui(scan, &num))\n+\t\t\tif (strtoul_ui(scan, 10, &num))\n \t\t\t\tdie(emsg_invalid_context_len, scan);\n \t\t\tswitch (arg[1]) {\n \t\t\tcase 'A':\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex 47d42ed..b3d4ace 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -227,6 +227,7 @@ static void read_index_info(int line_termination)\n \t\tchar *path_name;\n \t\tunsigned char sha1[20];\n \t\tunsigned int mode;\n+\t\tunsigned long ul;\n \t\tint stage;\n \n \t\t/* This reads lines formatted in one of three formats:\n@@ -249,9 +250,12 @@ static void read_index_info(int line_termination)\n \t\tif (buf.eof)\n \t\t\tbreak;\n \n-\t\tmode = strtoul(buf.buf, &ptr, 8);\n-\t\tif (ptr == buf.buf || *ptr != ' ')\n+\t\terrno = 0;\n+\t\tul = strtoul(buf.buf, &ptr, 8);\n+\t\tif (ptr == buf.buf || *ptr != ' '\n+\t\t    || errno || (unsigned int) ul != ul)\n \t\t\tgoto bad_line;\n+\t\tmode = ul;\n \n \t\ttab = strchr(ptr, '\\t');\n \t\tif (!tab || tab - ptr < 41)\n@@ -547,7 +551,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\t\tif (i+3 >= argc)\n \t\t\t\t\tdie(\"git-update-index: --cacheinfo <mode> <sha1> <path>\");\n \n-\t\t\t\tif ((sscanf(argv[i+1], \"%o\", &mode) != 1) ||\n+\t\t\t\tif ((strtoul_ui(argv[i+1], 8, &mode) != 1) ||\n \t\t\t\t    get_sha1_hex(argv[i+2], sha1) ||\n \t\t\t\t    add_cacheinfo(mode, sha1, argv[i+3], 0))\n \t\t\t\t\tdie(\"git-update-index: --cacheinfo\"\ndiff --git a/convert-objects.c b/convert-objects.c\nindex 4809f91..cf03bcf 100644\n--- a/convert-objects.c\n+++ b/convert-objects.c\n@@ -88,7 +88,7 @@ static int write_subdirectory(void *buffer, unsigned long size, const char *base\n \t\tunsigned int mode;\n \t\tchar *slash, *origpath;\n \n-\t\tif (!path || sscanf(buffer, \"%o\", &mode) != 1)\n+\t\tif (!path || strtoul_ui(buffer, 8, &mode) != 1)\n \t\t\tdie(\"bad tree conversion\");\n \t\tmode = convert_mode(mode);\n \t\tpath++;\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 139fc19..5f6a281 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -301,4 +301,17 @@ static inline int prefixcmp(const char *str, const char *prefix)\n \treturn strncmp(str, prefix, strlen(prefix));\n }\n \n+static inline int strtoul_ui(char const *s, int base, unsigned int *result)\n+{\n+\tunsigned long ul;\n+\tchar *p;\n+\n+\terrno = 0;\n+\tul = strtoul(s, &p, base);\n+\tif (errno || *p || p == s || (unsigned int) ul != ul)\n+\t\treturn -1;\n+\t*result = ul;\n+\treturn 0;\n+}\n+\n #endif\n-- \n1.5.1.rc3-dirty\n"},{"id":"39095","messageId":"7v4pnnqr9q.fsf@assigned-by-dhcp.cox.net","threadId":"7590","inReplyTo":"871witxicn.fsf@rho.meyering.net","subject":"Re: sscanf/strtoul: parse integers robustly","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-11T07:55:29Z","receivedAt":"2007-04-11T07:55:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 139fc19..5f6a281 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -301,4 +301,17 @@ static inline int prefixcmp(const char *str, const char *prefix)\n>  \treturn strncmp(str, prefix, strlen(prefix));\n>  }\n>  \n> +static inline int strtoul_ui(char const *s, int base, unsigned int *result)\n> +{\n> +\tunsigned long ul;\n> +\tchar *p;\n> +\n> +\terrno = 0;\n> +\tul = strtoul(s, &p, base);\n> +\tif (errno || *p || p == s || (unsigned int) ul != ul)\n> +\t\treturn -1;\n> +\t*result = ul;\n> +\treturn 0;\n> +}\n> +\n>  #endif\n\nWar on sscanf is fine, but I wonder if this is small enough to\nbe a good candidate for inlining.\n"},{"id":"39116","messageId":"87fy77t3we.fsf@rho.meyering.net","threadId":"7590","inReplyTo":"7v4pnnqr9q.fsf@assigned-by-dhcp.cox.net","subject":"Re: sscanf/strtoul: parse integers robustly","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2007-04-11T13:52:01Z","receivedAt":"2007-04-11T13:52:01Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n\n> Jim Meyering <jim@meyering.net> writes:\n>\n>> diff --git a/git-compat-util.h b/git-compat-util.h\n>> index 139fc19..5f6a281 100644\n>> --- a/git-compat-util.h\n>> +++ b/git-compat-util.h\n>> @@ -301,4 +301,17 @@ static inline int prefixcmp(const char *str, const char *prefix)\n>>  \treturn strncmp(str, prefix, strlen(prefix));\n>>  }\n>>\n>> +static inline int strtoul_ui(char const *s, int base, unsigned int *result)\n>> +{\n>> +\tunsigned long ul;\n>> +\tchar *p;\n>> +\n>> +\terrno = 0;\n>> +\tul = strtoul(s, &p, base);\n>> +\tif (errno || *p || p == s || (unsigned int) ul != ul)\n>> +\t\treturn -1;\n>> +\t*result = ul;\n>> +\treturn 0;\n>> +}\n>> +\n>>  #endif\n>\n> War on sscanf is fine, but I wonder if this is small enough to\n> be a good candidate for inlining.\n\nI don't care if it is actually inlined.\nI used \"inline\" because this function seems small enough that\nduplicating its code won't hurt, and because then I didn't need\nto bother with a separate prototype.  On the size front, it looks\nno larger than most of the other inline functions in that file.\n"},{"id":"39854","messageId":"37ce3db845caa21ba45c15d4f829ece1@pinky","threadId":"7590","inReplyTo":"871witxicn.fsf@rho.meyering.net","subject":"[PATCH] fix up strtoul_ui error handling","fromName":"Andy Whitcroft","fromEmail":"apw@shadowen.org","sentAt":"2007-04-19T02:08:15Z","receivedAt":"2007-04-19T02:08:15Z","isPatch":true,"sender":{"key":"apw@shadowen.org","avatar":"https://gravatar.com/avatar/d3088262854661a913ef35cc40fedcc270142d4461791142bc1ea0b2a4e2e147?d=mp&s=160"},"body":"\nTwo scanf() calls were converted to strtoul_ui() but the return\nvalues were not updated to match.  scanf() returns the number of\nmatched \"values\" which for this usage is 1 on success.  strtoul_ui()\nreturn 0 on success.  Update these call sites to match.\n\nSigned-off-by: Andy Whitcroft <apw@shadowen.org>\n---\n\tWithout this patch svnimport fails to add files as\n\tupdate-index --cacheinfo fails.\n---\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex 9205c9f..8f98991 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -627,7 +627,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\t\tif (i+3 >= argc)\n \t\t\t\t\tdie(\"git-update-index: --cacheinfo <mode> <sha1> <path>\");\n \n-\t\t\t\tif ((strtoul_ui(argv[i+1], 8, &mode) != 1) ||\n+\t\t\t\tif (strtoul_ui(argv[i+1], 8, &mode) ||\n \t\t\t\t    get_sha1_hex(argv[i+2], sha1) ||\n \t\t\t\t    add_cacheinfo(mode, sha1, argv[i+3], 0))\n \t\t\t\t\tdie(\"git-update-index: --cacheinfo\"\ndiff --git a/convert-objects.c b/convert-objects.c\nindex cf03bcf..cefbceb 100644\n--- a/convert-objects.c\n+++ b/convert-objects.c\n@@ -88,7 +88,7 @@ static int write_subdirectory(void *buffer, unsigned long size, const char *base\n \t\tunsigned int mode;\n \t\tchar *slash, *origpath;\n \n-\t\tif (!path || strtoul_ui(buffer, 8, &mode) != 1)\n+\t\tif (!path || strtoul_ui(buffer, 8, &mode))\n \t\t\tdie(\"bad tree conversion\");\n \t\tmode = convert_mode(mode);\n \t\tpath++;\n"},{"id":"39855","messageId":"7vtzvdayla.fsf@assigned-by-dhcp.cox.net","threadId":"7590","inReplyTo":"37ce3db845caa21ba45c15d4f829ece1@pinky","subject":"Re: [PATCH] fix up strtoul_ui error handling","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-19T02:26:41Z","receivedAt":"2007-04-19T02:26:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}