On Fri, Oct 17, 2008 at 11:17:48AM -0700, Dean Hildebrand wrote:
> a) Reduce nesting level of loops.
> b) Use correct data types.
> c) Use nloc and nserv instead of n and m variable names.
> d) Try to clean up formatting of debugging statements.
> e) Move while loops to for loops.
Does this also fix a possible infinite loop in the inner loop?
--b.
>
> Signed-off-by: Dean Hildebrand <dhildeb@xxxxxxxxxx>
> ---
> fs/nfs/nfs4xdr.c | 71 +++++++++++++++++++++++++++++-------------------------
> 1 files changed, 38 insertions(+), 33 deletions(-)
>
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 5e59481..0e78fbd 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -2560,7 +2560,8 @@ out_eio:
>
> static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, struct nfs4_fs_locations *res)
> {
> - int n;
> + u32 nloc;
> + unsigned int i;
> __be32 *p;
> int status = -EIO;
>
> @@ -2574,53 +2575,57 @@ static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, st
> if (unlikely(status != 0))
> goto out;
> READ_BUF(4);
> - READ32(n);
> - if (n <= 0)
> + READ32(nloc);
> + if (nloc <= 0)
> goto out_eio;
> -
> - if (n > NFS4_FS_LOCATIONS_MAXENTRIES) {
> - dprintk("%s: using first %u of %d fs locations\n",
> - __func__, NFS4_FS_LOCATIONS_MAXENTRIES, n);
> - n = NFS4_FS_LOCATIONS_MAXENTRIES;
> + if (nloc > NFS4_FS_LOCATIONS_MAXENTRIES) {
> + dprintk("%s: using first %u of %u fs locations\n",
> + __func__, NFS4_FS_LOCATIONS_MAXENTRIES, nloc);
> + nloc = NFS4_FS_LOCATIONS_MAXENTRIES;
> } else {
> - dprintk("%s: using %d fs locations\n",
> - __func__, n);
> + dprintk("%s: using %u fs locations\n",
> + __func__, nloc);
> }
>
> res->nlocations = 0;
> - while (res->nlocations < n) {
> - u32 m;
> - struct nfs4_fs_location *loc = &res->locations[res->nlocations];
> + for (i = 0; i < nloc; i++) {
> + u32 nserv;
> + unsigned int totalserv, j;
> + struct nfs4_fs_location *loc = &res->locations[i];
>
> READ_BUF(4);
> - READ32(m);
> + READ32(nserv);
> +
> + totalserv = nserv;
> + if (nserv > NFS4_FS_LOCATION_MAXSERVERS) {
> + dprintk("%s: Using first %u of %u servers "
> + "returned for location %u\n",
> + __func__, NFS4_FS_LOCATION_MAXSERVERS,
> + nserv, i);
> + nserv = NFS4_FS_LOCATION_MAXSERVERS;
> + }
>
> loc->nservers = 0;
> - dprintk("%s: servers ", __func__);
> - while (loc->nservers < m) {
> + dprintk("%s: Servers ", __func__);
> + for (j = 0; j < nserv; j++) {
> struct nfs4_string *server = &loc->servers[loc->nservers];
> status = decode_opaque_inline(xdr, &server->len, &server->data);
> if (unlikely(status != 0))
> goto out_eio;
> dprintk("%s ", server->data);
> - if (loc->nservers < NFS4_FS_LOCATION_MAXSERVERS)
> - loc->nservers++;
> - else {
> - unsigned int i;
> - dprintk("%s: using first %u of %u servers "
> - "returned for location %u\n",
> - __func__,
> - NFS4_FS_LOCATION_MAXSERVERS,
> - m, res->nlocations);
> - for (i = loc->nservers; i < m; i++) {
> - unsigned int len;
> - char *data;
> - status = decode_opaque_inline(xdr, &len, &data);
> - if (unlikely(status != 0))
> - goto out_eio;
> - }
> - }
> + loc->nservers++;
> + }
> + dprintk("\n");
> +
> + /* Decode and ignore overflow servers */
> + for (j = loc->nservers; j < totalserv; j++) {
> + unsigned int len;
> + char *data;
> + status = decode_opaque_inline(xdr, &len, &data);
> + if (unlikely(status != 0))
> + goto out_eio;
> }
> +
> status = decode_pathname(xdr, &loc->rootpath);
> if (unlikely(status != 0))
> goto out_eio;
> --
> 1.5.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
