[openib-general] Re: [patch] userspace/management/diags/src/sminfo.c - cmdline processing fix

[Hal Rosenstock]

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 <sashak at voltaire.com>
> 
> 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() );