[PATCH] mount: redo nfs error printing, fix memory leak (0 bytes)

[Denys Vlasenko]

On Wednesday 27 October 2010 15:33, Alexander Shishkin wrote:
> -/* Convert each NFSERR_BLAH into EBLAH */
> +/* Convert each NFSERR_BLAH into EBLAH, null-terminated so that it
> + * can be searched with strchr() */
>  static const uint8_t nfs_err_stat[] = {
>  	 1,  2,  5,  6, 13, 17,
>  	19, 20, 21, 22, 27, 28,
> -	30, 63, 66, 69, 70, 71
> +	30, 63, 66, 69, 70, 71, 0
>  };
>  #if ( \
>  	EPERM | ENOENT      | EIO      | ENXIO | EACCES| EEXIST | \
> @@ -768,15 +769,18 @@ static const nfs_err_type nfs_err_errnum[] = {
>  	ENODEV, ENOTDIR     , EISDIR   , EINVAL, EFBIG , ENOSPC,
>  	EROFS , ENAMETOOLONG, ENOTEMPTY, EDQUOT, ESTALE, EREMOTE
>  };
> -static char *nfs_strerror(int status)
> +
> +static void nfs_error_msg(char *hostname, char *path, unsigned int status)
>  {
> -	int i;
> +	uint8_t *r;
> +	char *reason = "unknown nfs status";
>  
> -	for (i = 0; i < ARRAY_SIZE(nfs_err_stat); i++) {
> -		if (nfs_err_stat[i] == status)
> -			return strerror(nfs_err_errnum[i]);
> -	}
> -	return xasprintf("unknown nfs status return value: %d", status);
> +	r = strchr(nfs_err_stat, status);

Bug. For example, status 257 will be interpreted as status 1
due to truncation to char in call to strchr.

> +	if (r)
> +		reason = strerror(nfs_err_errnum[r - nfs_err_stat]);
> +
> +	bb_error_msg("%s:%s failed, reason given by server: %s (%d)", hostname, path,
> +		     reason, status);
>  }
>  
>  static bool_t xdr_fhandle(XDR *xdrs, fhandle objp)
> @@ -1457,9 +1461,7 @@ static NOINLINE int nfsmount(struct mntent *mp, long vfsflags, char *filteropts)
>  
>  	if (nfsvers == 2) {
>  		if (status.nfsv2.fhs_status != 0) {
> -			bb_error_msg("%s:%s failed, reason given by server: %s",
> -				hostname, pathname,
> -				nfs_strerror(status.nfsv2.fhs_status));
> +			nfs_error_msg(hostname, pathname, status.nfsv2.fhs_status);
>  			goto fail;
>  		}

Hmmm... it is possible to fold "!= 0" check into nfs_error_msg(),
so that code looks simply like this:

	if (emit_nfs_error_msg(hostname, pathname, status.nfsv2.fhs_status))
		goto fail;  /* fhs_status != 0, message was printed */

...
>  		if (status.nfsv3.fhs_status != 0) {
> -			bb_error_msg("%s:%s failed, reason given by server: %s",
> -				hostname, pathname,
> -				nfs_strerror(status.nfsv3.fhs_status));
> +			nfs_error_msg(hostname, pathname, status.nfsv3.fhs_status);
>  			goto fail;
>  		}

in both places -> likely smaller code.

-- 
vda