Re: [Patch 1/1] CLD: Introduce the "New CLD" API

[Jeff Garzik <jeff@garzik.org>]

On 02/07/2010 02:15 PM, Pete Zaitcev wrote:
> +	case CREQ_MKDIR:
> +		rc = 0;
> +		fhp = ncld_open(nsp, creq.path,
> +				COM_DIRECTORY | COM_CREATE | COM_EXCL,&error,
> +				0, NULL, NULL);
> +		if (fhp)
> +			ncld_close(fhp);
> +		else
> +			rc = error;
> +		break;
> +
[...]
> -	}
> -	if (tcode != TC_OK) {
> -		fprintf(stderr, "thread startup failed\n");
> +	nsp = ncld_sess_open(dr->host, dr->port,&error, sess_event, NULL,
> +			     "cldcli", "cldcli");
> +	if (!nsp) {
> +		if (error<  1000) {
> +			fprintf(stderr, TAG ": cannot open CLD session: %s\n",
> +				strerror(error));
> +		} else {
> +			fprintf(stderr, TAG ": cannot open CLD session: %d\n",
> +				error);
> +		}
>   		return 1;
>   	}


A point of style:  the 'p' suffix is discouraged.  It is too much like 
the much-maligned Hungarian notation, which is odious in part because it 
duplicates information found in the type definition.  In this case, 'p' 
duplicates the indication that it is a pointer.

The return value from ncld_sess_open() should be named 'ns' or 'sess' or 
similar.  The return value from ncld_open() should be named 'fh' or similar.

	Jeff