[ofw] RE: ndinstall patch

Sean Hefty sean.hefty at intel.com
Thu Jul 16 14:26:36 PDT 2009


>+#define ND_DEFAULT_PROVIDER "ibal"

We can drop this.  The names are given in the provider_name array, which is
referenced by the global index variable that defaults to 0 (indicating IBAL).
To change the default provider from ibal to winverbs, we only need to initialize
index to 2.

>@@ -97,14 +99,25 @@
> static void
> show_usage (char *progname)
> {
>-	printf("usage: %s\n", progname);
>-	printf("\tRemove, install, and list OFA NetworkDirect providers\n\n");
>-	printf("\t[-[i|r] provider_name]  Install/remove the specified
>provider\n");
>-	printf("\t[-d]                    Install/remove debug version of
>provider\n");
>-	printf("\t[-l]                    list OFA ND providers\n");
>-	printf("\tprovider_name must be one of the following:\n");
>+	char drv[_MAX_DRIVE];
>+	char dir[_MAX_DIR];
>+	char fname[_MAX_FNAME];
>+	char ext[_MAX_EXT];
>+
>+	_splitpath(progname,drv,dir,fname,ext); // sad basename() equivalent.
>+
>+	printf("usage: %s%s\n",fname,ext);

This seems a little much to me...
 
>+	printf("\tRemove, install and list OFA NetworkDirect providers\n\n");
>+	printf("\t[-[i|r] [provider]] Install/remove the specified/default"
>+			" provider\n");
>+	printf("\t[-d]                Install/remove debug version of
>provider\n");
>+	printf("\t[-l]                list OFA ND providers\n");
>+	printf("\t[-q]                Suppress default listing of providers\n");
>+	printf("\t[-h]                This text\n");
>+	printf("\tprovider must be one of the following names:\n");
> 	printf("\t\tibal\n");
> 	printf("\t\twinverbs\n");
>+	printf("\t\t<blank> use the default ND provider '%s'\n",
>ND_DEFAULT_PROVIDER);

ND_DEFAULT_PROVIDER would become provider_name[index]

> }
>
>
>@@ -257,6 +270,9 @@
> {
> 	int i;
>
>+	if ( !name )
>+		name = ND_DEFAULT_PROVIDER;

This can be simplified to:

if (!name)
	return 0;

which will leave index set to the default.  Or change these checks in main:

	if (install || remove || get_prov_index(optarg)) {

to

	if (install || remove || (optarg && get_prov_index(optarg))) {

>+
> 	for (i = 0; i < MAX_INDEX; i++) {
> 		if (!_stricmp(provider_name[i], name)) {
> 			index = i;
>@@ -269,12 +285,14 @@
> int __cdecl main (int argc, char *argv[])
> {
> 	int ret=0, op;
>-	int install = 0, remove = 0, debug = 0;
>+	int install = 0, remove = 0, debug = 0, quiet = 0;

It seems like quiet could become a global value to suppress all output.  Now
that the program returns a proper status value, that can be used in place of
displayed output.

This would imply replacing all printf statements with a macro that checks quiet
before displaying any output.

- Sean




More information about the ofw mailing list