On Thu, 17 Jun 2010 16:19:43 +0300 (IDT) Alexander Nezhinsky <alexandern at voltaire.com> wrote: > When a numerical command line param value is out of range or contains garbage > (both tgtd and tgtadm), this remains unchecked in many cases. > In some of them 0 value will be used, which may lead to unpredicted or > "silently" faulty behavior. > > This fix defines a new function str_to_uit64() which uses strtoull() and > detects its errors properly, by checking 1) errno (range errors) and > 2) comparing endptr to the original argument (nun-numerical garbage); > must check both. > > str_to_uit64() is declared static twice, in tgtd.c and tgtadm.c, as > tgtadm does not uses util.c, where the natural place of this function > would be otherwise. > > It returns the converted integer value directly, while error is returned > through a pointer argument. Although the opposite arrangement may seem > intuitive, this one allows free casting of the returned value to various > types. Return type uint64 is choosen to cover all possible cases. > min and max arguments support range checking. To use the natural integer > ranges, use ULLONG_MAX, ULONG_MAX, USHRT_MAX etc. > > Signed-off-by: Alexander Nezhinsky <alexandern at voltaire.com> I'm for the strict checking however two comments. - casting a returned value looks ugly. - duplicating the same function isn't nice. How about something like this? diff --git a/usr/tgtadm.c b/usr/tgtadm.c index 28ed754..2282c86 100644 --- a/usr/tgtadm.c +++ b/usr/tgtadm.c @@ -471,8 +471,8 @@ int main(int argc, char **argv) mode = str_to_mode(optarg); break; case 't': - tid = strtol(optarg, NULL, 10); - if (errno == EINVAL) { + tid = str_to_val(optarg, int, 0, INT_MAX, &rc); + if (rc == EINVAL) { eprintf("invalid tid '%s'\n", optarg); exit(EINVAL); } diff --git a/usr/util.h b/usr/util.h index 6e1fd6f..fc406b1 100644 --- a/usr/util.h +++ b/usr/util.h @@ -156,4 +156,17 @@ struct signalfd_siginfo { }; #endif +#define str_to_val(str, type, min, max, err_ptr) \ +({ \ + char *ptr; \ + int ret = 0; \ + typeof(type) v = strtoull(str, &ptr, 0); \ + if (errno || ptr == str) \ + ret = EINVAL; \ + if (v < min|| v > max) \ + ret = ERANGE; \ + *err_ptr = ret; \ + v; \ +}) \ + #endif -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html |