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
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to