[stgt] [PATCH] improved conversion and checking of numerical cmd line args

[FUJITA Tomonori]

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