linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: trond.myklebust@netapp.com
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 05/13] NFS: Clean up debugging in decode_pathname()
Date: Wed, 15 Feb 2012 16:35:25 -0500	[thread overview]
Message-ID: <20120215213525.3254.55844.stgit@ellison.1015granger.net> (raw)
In-Reply-To: <20120215213336.3254.98936.stgit@ellison.1015granger.net>

I noticed recently that decode_attr_fs_locations() is not generating
very pretty debugging output.  The pathname components each appear on
a separate line of output, though that does not appear to be the
intended display behavior.  The preferred way to generate continued
lines of output on the console is to use pr_cont().

Note that incoming pathname4 components contain a string that is not
necessarily NUL-terminated.  I did actually see some trailing garbage
on the console.  In addition to correcting the line continuation
problem, add a string precision format specifier to ensure that each
component string is displayed properly, and that vsnprintf() does
not Oops.

Someone pointed out that allowing incoming network data to possibly
generate a console line of unbounded length may not be such a good
idea.  Since this output will rarely be enabled, and there is a hard
upper bound (NFS4_PATHNAME_MAXCOMPONENTS) in our implementation, this
is probably not a major concern.

It might be useful to additionally sanity-check the length of each
incoming component, however.  RFC 3530bis15 does not suggest a maximum
number of UTF-8 characters per component for either the pathname4 or
component4 types.  However, we could invent one that is appropriate
for our implementation.

Another possibility is to scrap all of this and print these pathnames
in upper layers after a reasonable amount of sanity checking in the
XDR layer.  This would give us an opportunity to allocate a full
buffer so that the whole pathname would be output via a single
dprintk.

Introduced by commit 7aaa0b3b: "NFSv4: convert fs-locations-components
to conform to RFC3530," (June 9, 2006).

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs4xdr.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 51ef966..606852a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -3512,16 +3512,17 @@ static int decode_pathname(struct xdr_stream *xdr, struct nfs4_pathname *path)
 	n = be32_to_cpup(p);
 	if (n == 0)
 		goto root_path;
-	dprintk("path ");
+	dprintk("pathname4: ");
 	path->ncomponents = 0;
 	while (path->ncomponents < n) {
 		struct nfs4_string *component = &path->components[path->ncomponents];
 		status = decode_opaque_inline(xdr, &component->len, &component->data);
 		if (unlikely(status != 0))
 			goto out_eio;
-		if (path->ncomponents != n)
-			dprintk("/");
-		dprintk("%s", component->data);
+		if (unlikely(nfs_debug & NFSDBG_XDR))
+			pr_cont("%s%.*s ",
+				(path->ncomponents != n ? "/ " : ""),
+				component->len, component->data);
 		if (path->ncomponents < NFS4_PATHNAME_MAXCOMPONENTS)
 			path->ncomponents++;
 		else {
@@ -3530,14 +3531,13 @@ static int decode_pathname(struct xdr_stream *xdr, struct nfs4_pathname *path)
 		}
 	}
 out:
-	dprintk("\n");
 	return status;
 root_path:
 /* a root pathname is sent as a zero component4 */
 	path->ncomponents = 1;
 	path->components[0].len=0;
 	path->components[0].data=NULL;
-	dprintk("path /\n");
+	dprintk("pathname4: /\n");
 	goto out;
 out_eio:
 	dprintk(" status %d", status);
@@ -3559,7 +3559,7 @@ static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, st
 	status = 0;
 	if (unlikely(!(bitmap[0] & FATTR4_WORD0_FS_LOCATIONS)))
 		goto out;
-	dprintk("%s: fsroot ", __func__);
+	dprintk("%s: fsroot:\n", __func__);
 	status = decode_pathname(xdr, &res->fs_path);
 	if (unlikely(status != 0))
 		goto out;
@@ -3580,7 +3580,7 @@ static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t *bitmap, st
 		m = be32_to_cpup(p);
 
 		loc->nservers = 0;
-		dprintk("%s: servers ", __func__);
+		dprintk("%s: servers:\n", __func__);
 		while (loc->nservers < m) {
 			struct nfs4_string *server = &loc->servers[loc->nservers];
 			status = decode_opaque_inline(xdr, &server->len, &server->data);


  parent reply	other threads:[~2012-02-15 21:35 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-15 21:34 [PATCH 00/13] For 3.4 (try 2) Chuck Lever
2012-02-15 21:34 ` [PATCH 01/13] NFS: Make nfs_cache_array.size a signed integer Chuck Lever
2012-02-16 19:52   ` Myklebust, Trond
2012-02-16 20:09     ` Chuck Lever
2012-02-16 20:34       ` Myklebust, Trond
2012-02-16 20:53         ` Chuck Lever
2012-02-15 21:35 ` [PATCH 02/13] NFS: Fix comparison signage warnings with slot ID computations Chuck Lever
2012-02-16 19:52   ` Myklebust, Trond
2012-02-16 20:10     ` Chuck Lever
2012-02-15 21:35 ` [PATCH 03/13] SUNRPC: Use KERN_DEFAULT for debugging printk's Chuck Lever
2012-02-15 22:05   ` Malahal Naineni
2012-02-16 19:54   ` Myklebust, Trond
2012-02-15 21:35 ` [PATCH 04/13] nfs: Clean up debugging in nfs_follow_mountpoint() Chuck Lever
2012-02-15 21:35 ` Chuck Lever [this message]
2012-02-15 21:35 ` [PATCH 06/13] NFS: Add debugging messages to NFSv4's CLOSE procedure Chuck Lever
2012-02-15 21:35 ` [PATCH 07/13] NFS: Reduce debugging noise from encode_compound_hdr Chuck Lever
2012-02-16 20:04   ` Myklebust, Trond
2012-02-16 20:11     ` Chuck Lever
2012-02-15 21:35 ` [PATCH 08/13] SUNRPC: Add API to acquire source address Chuck Lever
2012-02-15 22:12   ` Malahal Naineni
2012-02-15 22:17     ` Chuck Lever
2012-02-16 20:07   ` Myklebust, Trond
2012-02-16 20:13     ` Chuck Lever
2012-02-15 21:36 ` [PATCH 09/13] NFS: Add a client-side function to display NFS file handles Chuck Lever
2012-02-15 21:36 ` [PATCH 10/13] NFS: Save root file handle in nfs_server Chuck Lever
2012-02-15 21:36 ` [PATCH 11/13] NFS: Simplify arguments of encode_renew() Chuck Lever
2012-02-15 21:36 ` [PATCH 12/13] NFS: Introduce NFS_ATTR_FATTR_V4_LOCATIONS Chuck Lever
2012-02-15 21:36 ` [PATCH 13/13] NFS: Request fh_expire_type attribute in "server caps" operation Chuck Lever
2012-02-20 15:01   ` Matthew Treinish
  -- strict thread matches above, loose matches on Subject: below --
2012-02-15 19:41 [PATCH 00/13] For 3.4 Chuck Lever
2012-02-15 19:42 ` [PATCH 05/13] NFS: Clean up debugging in decode_pathname() Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120215213525.3254.55844.stgit@ellison.1015granger.net \
    --to=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@netapp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).