* Re: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics
@ 2010-12-20 21:57 Jay Fenlason
[not found] ` <20101220215735.GA4020-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jay Fenlason @ 2010-12-20 21:57 UTC (permalink / raw)
To: Ira Weiny
Cc: Sasha Khapyorsky,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Sorry if you get this more than once, I seem to have outgoing mail misconfiguration here.
On Wed, Dec 15, 2010 at 11:47:09AM -0800, Ira Weiny wrote:
>
> Verify timeout value specified to diagnostics
>
>
> Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org>
> ---
> infiniband-diags/src/ibdiag_common.c | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdia
+g_common.c
> index 99861f1..8ccf2fc 100644
> --- a/infiniband-diags/src/ibdiag_common.c
> +++ b/infiniband-diags/src/ibdiag_common.c
> @@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg)
> ibd_dest_type = IB_DEST_GUID;
> break;
> case 't':
> - val = strtoul(optarg, 0, 0);
> - madrpc_set_timeout(val);
> - ibd_timeout = val;
> + val = (int)strtol(optarg, NULL, 0);
> + if (val > 0) {
> + madrpc_set_timeout(val);
> + ibd_timeout = val;
> + } else
> + IBERROR("Invalid timeout \"%s\". Timeout requires a "
> + "positive integer value.", optarg);
> break;
> case 's':
> /* srcport is not required when resolving via IB_DEST_LID */
> --
> 1.5.4.5
This only partially detects invalid timeouts. For example, timeouts
of "123skidoo" or 1234534587347895789457897897894578978912902393 will
be accepted but will not work as expected. To fully test the
conversion, you need to do something more like:
long val;
char *endp;
...
errno = 0;
val = strtol ( optarg, &endp 0 );
if ( errno || ( endp && *endp != '\0' ) || val < 0 || val > INT_MAX ) {
/* invalid timeout detected */
...
All of the tests are required to detect invalid inputs.
-- JF
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread[parent not found: <20101220215735.GA4020-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* [PATCH] Further timeout paramater verification (Was: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics) [not found] ` <20101220215735.GA4020-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2010-12-22 19:47 ` Ira Weiny [not found] ` <20101222114707.c6ec051f.weiny2-i2BcT+NCU+M@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: Ira Weiny @ 2010-12-22 19:47 UTC (permalink / raw) To: Jay Fenlason Cc: Sasha Khapyorsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Mon, 20 Dec 2010 13:57:36 -0800 Jay Fenlason <fenlason-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > Sorry if you get this more than once, I seem to have outgoing mail misconfiguration here. > > On Wed, Dec 15, 2010 at 11:47:09AM -0800, Ira Weiny wrote: > > > > Verify timeout value specified to diagnostics > > > > > > Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> > > --- > > infiniband-diags/src/ibdiag_common.c | 10 +++++++--- > > 1 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdia > +g_common.c > > index 99861f1..8ccf2fc 100644 > > --- a/infiniband-diags/src/ibdiag_common.c > > +++ b/infiniband-diags/src/ibdiag_common.c > > @@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg) > > ibd_dest_type = IB_DEST_GUID; > > break; > > case 't': > > - val = strtoul(optarg, 0, 0); > > - madrpc_set_timeout(val); > > - ibd_timeout = val; > > + val = (int)strtol(optarg, NULL, 0); > > + if (val > 0) { > > + madrpc_set_timeout(val); > > + ibd_timeout = val; > > + } else > > + IBERROR("Invalid timeout \"%s\". Timeout requires a " > > + "positive integer value.", optarg); > > break; > > case 's': > > /* srcport is not required when resolving via IB_DEST_LID */ > > -- > > 1.5.4.5 > > This only partially detects invalid timeouts. For example, timeouts > of "123skidoo" or 1234534587347895789457897897894578978912902393 will > be accepted but will not work as expected. To fully test the > conversion, you need to do something more like: > > long val; > char *endp; > > ... > errno = 0; > val = strtol ( optarg, &endp 0 ); > if ( errno || ( endp && *endp != '\0' ) || val < 0 || val > INT_MAX ) { > /* invalid timeout detected */ > ... > > All of the tests are required to detect invalid inputs. > > -- JF Patch below, Ira From: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> Date: Wed, 22 Dec 2010 11:40:33 -0800 Subject: [PATCH] Further timeout paramater verification suggested by Jay Fenlason <fenlason-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> --- infiniband-diags/src/ibdiag_common.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-) diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdiag_common.c index 8ccf2fc..8f5eec3 100644 --- a/infiniband-diags/src/ibdiag_common.c +++ b/infiniband-diags/src/ibdiag_common.c @@ -138,7 +138,8 @@ void ibdiag_show_usage() static int process_opt(int ch, char *optarg) { - int val; + char *endp; + long val; switch (ch) { case 'h': @@ -175,13 +176,16 @@ static int process_opt(int ch, char *optarg) ibd_dest_type = IB_DEST_GUID; break; case 't': - val = (int)strtol(optarg, NULL, 0); - if (val > 0) { - madrpc_set_timeout(val); - ibd_timeout = val; - } else + errno = 0; + val = strtol(optarg, &endp, 0); + if (errno || (endp && *endp != '\0') || val <= 0 || + val > INT_MAX) IBERROR("Invalid timeout \"%s\". Timeout requires a " - "positive integer value.", optarg); + "positive integer value < %d.", optarg, INT_MAX); + else { + madrpc_set_timeout((int)val); + ibd_timeout = (int)val; + } break; case 's': /* srcport is not required when resolving via IB_DEST_LID */ -- 1.5.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <20101222114707.c6ec051f.weiny2-i2BcT+NCU+M@public.gmane.org>]
* Re: [PATCH] Further timeout paramater verification (Was: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics) [not found] ` <20101222114707.c6ec051f.weiny2-i2BcT+NCU+M@public.gmane.org> @ 2011-01-02 15:53 ` Sasha Khapyorsky 0 siblings, 0 replies; 5+ messages in thread From: Sasha Khapyorsky @ 2011-01-02 15:53 UTC (permalink / raw) To: Ira Weiny Cc: Jay Fenlason, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 11:47 Wed 22 Dec , Ira Weiny wrote: > On Mon, 20 Dec 2010 13:57:36 -0800 > Jay Fenlason <fenlason-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > > > Sorry if you get this more than once, I seem to have outgoing mail misconfiguration here. > > > > On Wed, Dec 15, 2010 at 11:47:09AM -0800, Ira Weiny wrote: > > > > > > Verify timeout value specified to diagnostics > > > > > > > > > Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> > > > --- > > > infiniband-diags/src/ibdiag_common.c | 10 +++++++--- > > > 1 files changed, 7 insertions(+), 3 deletions(-) > > > > > > diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdia > > +g_common.c > > > index 99861f1..8ccf2fc 100644 > > > --- a/infiniband-diags/src/ibdiag_common.c > > > +++ b/infiniband-diags/src/ibdiag_common.c > > > @@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg) > > > ibd_dest_type = IB_DEST_GUID; > > > break; > > > case 't': > > > - val = strtoul(optarg, 0, 0); > > > - madrpc_set_timeout(val); > > > - ibd_timeout = val; > > > + val = (int)strtol(optarg, NULL, 0); > > > + if (val > 0) { > > > + madrpc_set_timeout(val); > > > + ibd_timeout = val; > > > + } else > > > + IBERROR("Invalid timeout \"%s\". Timeout requires a " > > > + "positive integer value.", optarg); > > > break; > > > case 's': > > > /* srcport is not required when resolving via IB_DEST_LID */ > > > -- > > > 1.5.4.5 > > > > This only partially detects invalid timeouts. For example, timeouts > > of "123skidoo" or 1234534587347895789457897897894578978912902393 will > > be accepted but will not work as expected. To fully test the > > conversion, you need to do something more like: > > > > long val; > > char *endp; > > > > ... > > errno = 0; > > val = strtol ( optarg, &endp 0 ); > > if ( errno || ( endp && *endp != '\0' ) || val < 0 || val > INT_MAX ) { > > /* invalid timeout detected */ > > ... > > > > All of the tests are required to detect invalid inputs. > > > > -- JF > > > Patch below, > Ira > > > From: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> > Date: Wed, 22 Dec 2010 11:40:33 -0800 > Subject: [PATCH] Further timeout paramater verification > > suggested by Jay Fenlason <fenlason-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> > --- > infiniband-diags/src/ibdiag_common.c | 18 +++++++++++------- > 1 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdiag_common.c > index 8ccf2fc..8f5eec3 100644 > --- a/infiniband-diags/src/ibdiag_common.c > +++ b/infiniband-diags/src/ibdiag_common.c > @@ -138,7 +138,8 @@ void ibdiag_show_usage() > > static int process_opt(int ch, char *optarg) > { > - int val; > + char *endp; > + long val; > > switch (ch) { > case 'h': > @@ -175,13 +176,16 @@ static int process_opt(int ch, char *optarg) > ibd_dest_type = IB_DEST_GUID; > break; > case 't': > - val = (int)strtol(optarg, NULL, 0); > - if (val > 0) { > - madrpc_set_timeout(val); > - ibd_timeout = val; > - } else > + errno = 0; > + val = strtol(optarg, &endp, 0); > + if (errno || (endp && *endp != '\0') || val <= 0 || > + val > INT_MAX) When using INT_MAX <limits.h> header should be included to prevent src/ibdiag_common.c:182: error: ‘INT_MAX’ undeclared error. I'm fixing this. Applied. Thanks. Sasha > IBERROR("Invalid timeout \"%s\". Timeout requires a " > - "positive integer value.", optarg); > + "positive integer value < %d.", optarg, INT_MAX); > + else { > + madrpc_set_timeout((int)val); > + ibd_timeout = (int)val; > + } > break; > case 's': > /* srcport is not required when resolving via IB_DEST_LID */ > -- > 1.5.4.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] infiniband-diags: Verify timeout value specified to diagnostics
@ 2010-12-15 19:47 Ira Weiny
[not found] ` <20101215114709.82671602.weiny2-i2BcT+NCU+M@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Ira Weiny @ 2010-12-15 19:47 UTC (permalink / raw)
To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Verify timeout value specified to diagnostics
Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org>
---
infiniband-diags/src/ibdiag_common.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-diags/src/ibdiag_common.c
index 99861f1..8ccf2fc 100644
--- a/infiniband-diags/src/ibdiag_common.c
+++ b/infiniband-diags/src/ibdiag_common.c
@@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg)
ibd_dest_type = IB_DEST_GUID;
break;
case 't':
- val = strtoul(optarg, 0, 0);
- madrpc_set_timeout(val);
- ibd_timeout = val;
+ val = (int)strtol(optarg, NULL, 0);
+ if (val > 0) {
+ madrpc_set_timeout(val);
+ ibd_timeout = val;
+ } else
+ IBERROR("Invalid timeout \"%s\". Timeout requires a "
+ "positive integer value.", optarg);
break;
case 's':
/* srcport is not required when resolving via IB_DEST_LID */
--
1.5.4.5
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 5+ messages in thread[parent not found: <20101215114709.82671602.weiny2-i2BcT+NCU+M@public.gmane.org>]
* Re: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics [not found] ` <20101215114709.82671602.weiny2-i2BcT+NCU+M@public.gmane.org> @ 2010-12-20 15:31 ` Sasha Khapyorsky 0 siblings, 0 replies; 5+ messages in thread From: Sasha Khapyorsky @ 2010-12-20 15:31 UTC (permalink / raw) To: Ira Weiny; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 11:47 Wed 15 Dec , Ira Weiny wrote: > > Verify timeout value specified to diagnostics > > > Signed-off-by: Ira Weiny <weiny2-eSE4LqFkL++LYFxP40JT4w@public.gmane.org> Applied. Thanks. Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-02 15:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-20 21:57 [PATCH] infiniband-diags: Verify timeout value specified to diagnostics Jay Fenlason
[not found] ` <20101220215735.GA4020-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-12-22 19:47 ` [PATCH] Further timeout paramater verification (Was: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics) Ira Weiny
[not found] ` <20101222114707.c6ec051f.weiny2-i2BcT+NCU+M@public.gmane.org>
2011-01-02 15:53 ` Sasha Khapyorsky
-- strict thread matches above, loose matches on Subject: below --
2010-12-15 19:47 [PATCH] infiniband-diags: Verify timeout value specified to diagnostics Ira Weiny
[not found] ` <20101215114709.82671602.weiny2-i2BcT+NCU+M@public.gmane.org>
2010-12-20 15:31 ` Sasha Khapyorsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox