author | Jim Meyering <jim@meyering.net> | |
Mon, 12 Mar 2007 12:11:29 +0000 (13:11 +0100) | ||
committer | Junio C Hamano <junkio@cox.net> | |
Wed, 14 Mar 2007 08:37:50 +0000 (01:37 -0700) | ||
commit | 09f2825147d44bd0333f7f47d8ab3837f0848867 | |
tree | 2825425ab6d460ad07283dbc4fe4eae5aa33a8dc | tree | snapshot |
parent | 6cd7895feeaee3970b76988ed82b2c701a5b28df | commit | diff |
git-grep: don't use sscanf
If you use scanf or sscanf to parse integers, your code probably
accepts bogus inputs. For example, builtin-grep (aka git-grep) uses
sscanf(scan, "%u", &num) to parse the integer argument to -A, -B, -C.
Currently, "-C 1,000" and "-C 4294967297" are both treated just like
"-C 1":
$ git-grep -h -C 4294967297 juggle
out and you may find it easier to switch back and forth if you
juggle multiple lines of development simultaneously. Of
course, you will pay the price of more disk usage to hold
The obvious fix is to use strtoul instead. But using a bare strtoul is
too messy, at least when done properly, so I've added a wrapper function.
The new function in the patch below belongs elsewhere if it would be
useful in replacing any of the four remaining uses of sscanf.
One final note: With this change, I get a slightly different
diagnostic depending on the context size:
$ ./git-grep -h -C 4294967296 juggle
fatal: 4294967296: invalid context length argument
[Exit 128]
$ ./git-grep -h -C 4294967295 juggle
grep: 4294967295: invalid context length argument
[Exit 1]
A common convention that makes it easy to identify the source
of a diagnostic is to include the program name before the first ":".
Whether that should be "git" or "git-grep" is another question.
Using "grep" or "fatal" is misleading.
Signed-off-by: Jim Meyering <jim@meyering.net>
Signed-off-by: Junio C Hamano <junkio@cox.net>
If you use scanf or sscanf to parse integers, your code probably
accepts bogus inputs. For example, builtin-grep (aka git-grep) uses
sscanf(scan, "%u", &num) to parse the integer argument to -A, -B, -C.
Currently, "-C 1,000" and "-C 4294967297" are both treated just like
"-C 1":
$ git-grep -h -C 4294967297 juggle
out and you may find it easier to switch back and forth if you
juggle multiple lines of development simultaneously. Of
course, you will pay the price of more disk usage to hold
The obvious fix is to use strtoul instead. But using a bare strtoul is
too messy, at least when done properly, so I've added a wrapper function.
The new function in the patch below belongs elsewhere if it would be
useful in replacing any of the four remaining uses of sscanf.
One final note: With this change, I get a slightly different
diagnostic depending on the context size:
$ ./git-grep -h -C 4294967296 juggle
fatal: 4294967296: invalid context length argument
[Exit 128]
$ ./git-grep -h -C 4294967295 juggle
grep: 4294967295: invalid context length argument
[Exit 1]
A common convention that makes it easy to identify the source
of a diagnostic is to include the program name before the first ":".
Whether that should be "git" or "git-grep" is another question.
Using "grep" or "fatal" is misleading.
Signed-off-by: Jim Meyering <jim@meyering.net>
Signed-off-by: Junio C Hamano <junkio@cox.net>
builtin-grep.c | diff | blob | history |