[openib-general] Re: [patch] userspace/management/diags/src/sminfo.c - cmdline processing fix
Hi Sasha,
On Tue, 2006-01-17 at 05:10, Sasha Khapyorsky wrote:
> Hello Hal,
>
> There is small bug in sminfo's cmdline processing, this will segfault
> when option argument is missing (like 'sminfo -a'). The "fast and dirty"
> fix is inlined.
Thanks. Applied.
> The same problem exists with most diag tools, so I think we need to
> rework AGRBEGIN { ... } ARGEND stuff (actually remove it from
> libibcommon since it is used by diag tools only). I can do it if there
> are no objections.
I would welcome such a patch. Thanks.
-- Hal
> Regards,
> Sasha.
>
>
> This fast fix for invalid ARGF() usage in sminfo.c.
>
> Signed-off-by: Sasha Khapyorsky <[EMAIL PROTECTED]>
>
> Index: diags/src/sminfo.c
> ===
> -- diags/src/sminfo.c (revision 5017)
> +++ diags/src/sminfo.c(working copy)
> @@ -49,6 +49,8 @@
>
> #define IBERROR(fmt, args...)iberror(__FUNCTION__, fmt, ## args)
>
> +#define SAFE_ARGF() (*(argv+1) ? ARGF() : ( usage(), NULL ) )
> +
> static void
> iberror(const char *fn, char *msg, ...)
> {
> @@ -116,10 +118,10 @@
>
> ARGBEGIN {
> case 'C':
> - ca = ARGF();
> + ca = SAFE_ARGF();
> break;
> case 'P':
> - ca_port = strtoul(ARGF(), 0, 0);
> + ca_port = strtoul(SAFE_ARGF(), 0, 0);
> break;
> case 'd':
> ibdebug++;
> @@ -137,17 +139,17 @@
> dest_type = IB_DEST_GUID;
> break;
> case 't':
> - timeout = strtoul(ARGF(), 0, 0);
> + timeout = strtoul(SAFE_ARGF(), 0, 0);
> madrpc_set_timeout(timeout);
> break;
> case 'a':
> - act = strtoul(ARGF(), 0, 0);
> + act = strtoul(SAFE_ARGF(), 0, 0);
> break;
> case 's':
> - state = strtoul(ARGF(), 0, 0);
> + state = strtoul(SAFE_ARGF(), 0, 0);
> break;
> case 'p':
> - prio = strtoul(ARGF(), 0, 0);
> + prio = strtoul(SAFE_ARGF(), 0, 0);
> break;
> case 'V':
> fprintf(stderr, "%s %s\n", argv0, get_build_version() );
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] RE: [patch] userspace/management/diags/src/sminfo.c - cmdline processing fix
Hi Sasha,
Thanks. Applied.
I would welcome such a patch.
-- Hal
From: Sasha Khapyorsky [mailto:[EMAIL PROTECTED]
Sent: Tue 1/17/2006 5:10 AM
To: Hal Rosenstock
Cc: openib
Subject: [patch] userspace/management/diags/src/sminfo.c - cmdline processing
fix
Hello Hal,
There is small bug in sminfo's cmdline processing, this will segfault
when option argument is missing (like 'sminfo -a'). The "fast and dirty"
fix is inlined.
The same problem exists with most diag tools, so I think we need to
rework AGRBEGIN { ... } ARGEND stuff (actually remove it from
libibcommon since it is used by diag tools only). I can do it if there
are no objections.
Regards,
Sasha.
This fast fix for invalid ARGF() usage in sminfo.c.
Signed-off-by: Sasha Khapyorsky <[EMAIL PROTECTED]>
Index: diags/src/sminfo.c
===
--- diags/src/sminfo.c (revision 5017)
+++ diags/src/sminfo.c (working copy)
@@ -49,6 +49,8 @@
#define IBERROR(fmt, args...) iberror(__FUNCTION__, fmt, ## args)
+#define SAFE_ARGF() (*(argv+1) ? ARGF() : ( usage(), NULL ) )
+
static void
iberror(const char *fn, char *msg, ...)
{
@@ -116,10 +118,10 @@
ARGBEGIN {
case 'C':
- ca = ARGF();
+ ca = SAFE_ARGF();
break;
case 'P':
- ca_port = strtoul(ARGF(), 0, 0);
+ ca_port = strtoul(SAFE_ARGF(), 0, 0);
break;
case 'd':
ibdebug++;
@@ -137,17 +139,17 @@
dest_type = IB_DEST_GUID;
break;
case 't':
- timeout = strtoul(ARGF(), 0, 0);
+ timeout = strtoul(SAFE_ARGF(), 0, 0);
madrpc_set_timeout(timeout);
break;
case 'a':
- act = strtoul(ARGF(), 0, 0);
+ act = strtoul(SAFE_ARGF(), 0, 0);
break;
case 's':
- state = strtoul(ARGF(), 0, 0);
+ state = strtoul(SAFE_ARGF(), 0, 0);
break;
case 'p':
- prio = strtoul(ARGF(), 0, 0);
+ prio = strtoul(SAFE_ARGF(), 0, 0);
break;
case 'V':
fprintf(stderr, "%s %s\n", argv0, get_build_version() );
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] RE: [patch] userspace/management/diags/src/sminfo.c -cmdline processing fix
Hi Michael,
I believe this is largely historical. I will put this on the list TODO for the
diags and hopefully get to it in the not too distant future.
-- Hal
From: Michael S. Tsirkin [mailto:[EMAIL PROTECTED]
Sent: Tue 1/17/2006 5:30 AM
To: Sasha Khapyorsky
Cc: Hal Rosenstock; openib
Subject: Re: [patch] userspace/management/diags/src/sminfo.c -cmdline
processing fix
Quoting Sasha Khapyorsky <[EMAIL PROTECTED]>:
> Subject: [patch] userspace/management/diags/src/sminfo.c -cmdline processing
> fix
>
> Hello Hal,
>
> There is small bug in sminfo's cmdline processing, this will segfault
> when option argument is missing (like 'sminfo -a'). The "fast and dirty"
> fix is inlined.
>
> The same problem exists with most diag tools, so I think we need to
> rework AGRBEGIN { ... } ARGEND stuff (actually remove it from
> libibcommon since it is used by diag tools only). I can do it if there
> are no objections.
>
> Regards,
> Sasha.
>
>
> This fast fix for invalid ARGF() usage in sminfo.c.
>
> Signed-off-by: Sasha Khapyorsky <[EMAIL PROTECTED]>
BTW, why arent the diags using the standard getopt_long?
That would solve the problem above in a clean way and help us get rid of
the ARGxxx macros completely.
Hal?
--
MST
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [patch] userspace/management/diags/src/sminfo.c -cmdline processing fix
On 12:30 Tue 17 Jan , Michael S. Tsirkin wrote: > > BTW, why arent the diags using the standard getopt_long? > That would solve the problem above in a clean way and help us get rid of > the ARGxxx macros completely. Agree. Sasha. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [patch] userspace/management/diags/src/sminfo.c -cmdline processing fix
Quoting Sasha Khapyorsky <[EMAIL PROTECTED]>:
> Subject: [patch] userspace/management/diags/src/sminfo.c -cmdline processing
> fix
>
> Hello Hal,
>
> There is small bug in sminfo's cmdline processing, this will segfault
> when option argument is missing (like 'sminfo -a'). The "fast and dirty"
> fix is inlined.
>
> The same problem exists with most diag tools, so I think we need to
> rework AGRBEGIN { ... } ARGEND stuff (actually remove it from
> libibcommon since it is used by diag tools only). I can do it if there
> are no objections.
>
> Regards,
> Sasha.
>
>
> This fast fix for invalid ARGF() usage in sminfo.c.
>
> Signed-off-by: Sasha Khapyorsky <[EMAIL PROTECTED]>
BTW, why arent the diags using the standard getopt_long?
That would solve the problem above in a clean way and help us get rid of
the ARGxxx macros completely.
Hal?
--
MST
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
