Ethereal-dev: [Ethereal-dev] [patch] Major cleanup for NFSv4 dissection

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Mike Frisch <mfrisch@xxxxxxxxxx>
Date: Tue, 21 May 2002 16:35:24 -0400
I have done a spring house cleaning on the code in packet-nfs.c and
fixed all of the 'TODO's.  The changes are not well tested, but the code
compiles clean and I am able to capture/display NFSv4 traffic without
incident.

Mike.
Index: packet-nfs.c
===================================================================
RCS file: /cvsroot/ethereal/packet-nfs.c,v
retrieving revision 1.70
diff -u -r1.70 packet-nfs.c
--- packet-nfs.c	2002/05/21 10:17:18	1.70
+++ packet-nfs.c	2002/05/21 20:31:52
@@ -246,6 +246,8 @@
 static int hf_nfs_reclaim4 = -1;
 static int hf_nfs_length4 = -1;
 static int hf_nfs_changeid4 = -1;
+static int hf_nfs_changeid4_before = -1;
+static int hf_nfs_changeid4_after = -1;
 static int hf_nfs_nfstime4_seconds = -1;
 static int hf_nfs_nfstime4_nseconds = -1;
 static int hf_nfs_fsid4_major = -1;
@@ -4414,39 +4416,19 @@
 dissect_nfs_utf8string(tvbuff_t *tvb, int offset,
 	proto_tree *tree, int hf, char **string_ret)
 {
-	/* TODO: this needs to be fixed */
+	/* TODO: this dissector is subject to change; do not remove */
 	return dissect_rpc_string(tvb, tree, hf, offset, string_ret);
 }
 
 int
-dissect_nfs_linktext4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name" */
-	return dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_linktext4, 
-		NULL);
-}
-
-int
-dissect_nfs_specdata4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_specdata4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_specdata1, offset);
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_specdata2, offset);
 
 	return offset;
 }
 
-int
-dissect_nfs_clientid4(tvbuff_t *tvb, int offset,
-	proto_tree *tree)
-{
-	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_clientid4, offset);
-
-	return offset;
-}
-
 static const value_string names_ftype4[] = {
 	{	NF4REG,	"NF4REG"	},
 	{	NF4DIR,	"NF4DIR"	},
@@ -4460,55 +4442,22 @@
 	{ 0, NULL }
 };
 
-int
-dissect_nfs_component4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name" */
-	return dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_component4, 
-		NULL);
-}
-
-/* TODO: this function is not referenced at all. Remove it? */
-int
-dissect_nfs_reclaim4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name */
-	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_reclaim4, offset);
-	return offset;
-}
-
 int
-dissect_nfs_length4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_lock_owner4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name */
-	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_length4, offset);
-	return offset;
-}
-
-int
-dissect_nfs_opaque4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name);
-
-int
-dissect_nfs_lock_owner4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use paramter "name" */
 	proto_tree *newftree = NULL;
 	proto_item *fitem = NULL;
 
 	fitem = proto_tree_add_text(tree, tvb, offset, 4, "Owner");
 
-	if (fitem) {
+	if (fitem) 
+	{
 		newftree = proto_item_add_subtree(fitem, ett_nfs_lock_owner4);
 
-		if (newftree) {
-			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_clientid4,
-				offset);
-			offset = dissect_nfs_opaque4(tvb, offset, newftree, "Owner");
+		if (newftree) 
+		{
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_clientid4, offset);
+			offset = dissect_nfsdata(tvb, offset, newftree, hf_nfs_data);
 		}
 	}
 
@@ -4516,10 +4465,8 @@
 }
 
 int
-dissect_nfs_pathname4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_pathname4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use paramter "name" */
 	guint comp_count, i;
 	proto_item *fitem = NULL;
 	proto_tree *newftree = NULL;
@@ -4534,7 +4481,7 @@
 
 		if (newftree) {
 			for (i=0; i<comp_count; i++)
-				offset=dissect_nfs_component4(tvb, offset, newftree, "comp");
+				offset=dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 		}
 	}
 
@@ -4542,23 +4489,11 @@
 }
 
 int
-dissect_nfs_changeid4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_nfstime4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
-	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_changeid4, offset);
-	return offset;
-}
+	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_nfstime4_seconds, offset);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_nfstime4_nseconds, offset);
 
-int
-dissect_nfs_nfstime4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name" */
-	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_nfstime4_seconds, 
-		offset);
-	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_nfstime4_nseconds,
-		offset);
 	return offset;
 }
 
@@ -4582,14 +4517,13 @@
 	offset += 4;
 
 	if (set_it == SET_TO_CLIENT_TIME4)
-		offset = dissect_nfs_nfstime4(tvb, offset, tree, NULL);
+		offset = dissect_nfs_nfstime4(tvb, offset, tree);
 	
 	return offset;
 }
 
 int
-dissect_nfs_fsid4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name)
+dissect_nfs_fsid4(tvbuff_t *tvb, int offset, proto_tree *tree, char *name)
 {
 	proto_tree *newftree = NULL;
 	proto_item *fitem = NULL;
@@ -4611,38 +4545,23 @@
 }
 
 int
-dissect_nfs_acetype4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_nfsace4(tvbuff_t *tvb, int offset, packet_info *pinfo,
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_acetype4, offset);
-	return offset;
-}
-
-int
-dissect_nfs_aceflag4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name" */
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_aceflag4, offset);
-	return offset;
-}
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_acemask4, offset);
+	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_who, NULL);
 
-int
-dissect_nfs_acemask4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name" */
-	return dissect_rpc_uint32(tvb, tree, hf_nfs_acemask4, offset);
+	return offset;
 }
 
 int
-dissect_nfs_nfsace4(tvbuff_t *tvb, int offset,
+dissect_nfs_fattr4_acl(tvbuff_t *tvb, int offset, packet_info *pinfo,
 	proto_tree *tree, char *name)
 {
 	proto_tree *newftree = NULL;
 	proto_item *fitem = NULL;
-	int nextentry;
 
 	fitem = proto_tree_add_text(tree, tvb, offset, 0, "%s", name);
 
@@ -4651,22 +4570,8 @@
 	newftree = proto_item_add_subtree(fitem, ett_nfs_fsid4);
 
 	if (newftree == NULL) return offset;
-
-	nextentry = tvb_get_ntohl(tvb, offset);
-	offset = dissect_rpc_bool(tvb, newftree, hf_nfs_data_follows,
-		offset);
 
-	while (nextentry)
-	{
-		offset = dissect_nfs_acetype4(tvb, offset, newftree, "type");
-		offset = dissect_nfs_aceflag4(tvb, offset, newftree, "flag");
-		offset = dissect_nfs_acemask4(tvb, offset, newftree, 
-			"access_mask");
-		offset = dissect_nfs_utf8string(tvb, offset, newftree, 
-			hf_nfs_who, NULL);
-		nextentry = tvb_get_ntohl(tvb, offset);
-		offset += 4;
-	}
+	offset = dissect_rpc_list(tvb, pinfo, tree, offset, dissect_nfs_nfsace4);
 
 	return offset;
 }
@@ -4693,8 +4598,7 @@
 
 	if (newftree == NULL) return offset;
 
-	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_server, 
-		NULL);
+	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_server, NULL);
 
 	return offset;
 }
@@ -4715,11 +4619,10 @@
 
 	if (newftree == NULL) return offset;
 
-	offset = dissect_nfs_pathname4(tvb, offset, newftree, "fs_root");
+	offset = dissect_nfs_pathname4(tvb, offset, newftree);
 
 	nextentry = tvb_get_ntohl(tvb, offset);
-	offset = dissect_rpc_bool(tvb, newftree, hf_nfs_data_follows, 
-		offset);
+	offset = dissect_rpc_bool(tvb, newftree, hf_nfs_data_follows, offset);
 
 	while (nextentry)
 	{
@@ -4755,8 +4658,7 @@
 
 
 int
-dissect_nfs_fattr4_fh_expire_type(tvbuff_t *tvb, int offset, 
-	proto_tree *tree)
+dissect_nfs_fattr4_fh_expire_type(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
 	guint32 expire_type;
 	proto_item *expire_type_item = NULL;
@@ -4925,9 +4827,8 @@
 
 int
 dissect_nfs_attributes(tvbuff_t *tvb, int offset, packet_info *pinfo,
-	proto_tree *tree, char *name _U_, int type)
+	proto_tree *tree, int type)
 {
-	/* TODO: use parameter "name" */
 	guint32 bitmap_len;
 	proto_item *fitem = NULL;
 	proto_tree *newftree = NULL;
@@ -4992,7 +4893,7 @@
 					case FATTR4_SUPPORTED_ATTRS:
 						attr_vals_offset = dissect_nfs_attributes(tvb, 
 							attr_vals_offset, pinfo, attr_newftree, 
-							"fattr4_supported_attrs", FATTR4_BITMAP_ONLY);
+							FATTR4_BITMAP_ONLY);
 						break;
 						
 					case FATTR4_TYPE:
@@ -5006,8 +4907,8 @@
 						break;
 
 					case FATTR4_CHANGE:
-						attr_vals_offset = dissect_nfs_changeid4(tvb, 
-							attr_vals_offset, attr_newftree, "fattr4_change");
+						attr_vals_offset = dissect_rpc_uint64(tvb, attr_newftree, 
+							hf_nfs_changeid4, attr_vals_offset);
 						break;
 
 					case FATTR4_SIZE:
@@ -5055,8 +4956,8 @@
 						break;
 
 					case FATTR4_ACL:
-						attr_vals_offset = dissect_nfs_nfsace4(tvb, attr_vals_offset,
-							attr_newftree, "fattr4_acl");
+						attr_vals_offset = dissect_nfs_fattr4_acl(tvb, 
+							attr_vals_offset, pinfo, attr_newftree, "fattr4_acl");
 						break;
 
 					case FATTR4_ACLSUPPORT:
@@ -5214,7 +5115,7 @@
 
 					case FATTR4_RAWDEV:
 						attr_vals_offset = dissect_nfs_specdata4(tvb, 
-							attr_vals_offset, attr_newftree, "fattr4_rawdev");
+							attr_vals_offset, attr_newftree);
 						break;
 
 					case FATTR4_SPACE_AVAIL:
@@ -5251,7 +5152,7 @@
 					case FATTR4_TIME_METADATA:
 					case FATTR4_TIME_MODIFY:
 						attr_vals_offset = dissect_nfs_nfstime4(tvb, attr_vals_offset,
-							attr_newftree, "nfstime4");
+							attr_newftree);
 						break;
 
 					case FATTR4_TIME_ACCESS_SET:
@@ -5292,7 +5193,7 @@
 
 	if (newftree == NULL) return offset;
 
-	offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, name, 
+	offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, 
 		FATTR4_FULL_DISSECT);
 
 	offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_attrlist4);
@@ -5353,7 +5254,7 @@
 int
 dissect_nfs_open_owner4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	offset = dissect_nfs_clientid4(tvb, offset, tree);
+	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_clientid4, offset);
 	offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_open_owner4);
 
 	return offset;
@@ -5361,12 +5262,12 @@
 
 int
 dissect_nfs_open_claim_delegate_cur4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	offset = dissect_rpc_uint64(tvb, tree, 
 		hf_nfs_stateid4_delegate_stateid, offset);
-	offset = dissect_nfs_component4(tvb, offset, tree, "file");
+	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_component4, NULL);
+
 	return offset;
 }
 
@@ -5404,8 +5305,7 @@
 			switch(open_claim_type4)
 			{
 			case CLAIM_NULL:
-				offset = dissect_nfs_component4(tvb, offset, newftree, 
-					"file");
+				offset = dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 				break;
 
 			case CLAIM_PREVIOUS:
@@ -5415,12 +5315,11 @@
 
 			case CLAIM_DELEGATE_CUR:
 				offset = dissect_nfs_open_claim_delegate_cur4(tvb, offset,
-					newftree, "delegate_cur_info");
+					newftree);
 				break;
 
 			case CLAIM_DELEGATE_PREV:
-				offset = dissect_nfs_component4(tvb, offset, newftree, 
-					"file_delegate_prev");
+				offset = dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 				break;
 
 			default:
@@ -5434,9 +5333,8 @@
 
 int
 dissect_nfs_createhow4(tvbuff_t *tvb, int offset, packet_info *pinfo,
-	proto_tree *tree, char *name _U_)
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	guint mode;
 
 	/* This is intentional; we're using the same flags as NFSv3 */
@@ -5491,8 +5389,7 @@
 			switch(opentype4)
 			{
 			case OPEN4_CREATE:
-				offset = dissect_nfs_createhow4(tvb, offset, pinfo, newftree, 
-					"how");
+				offset = dissect_nfs_createhow4(tvb, offset, pinfo, newftree);
 				break;
 
 			default:
@@ -5505,12 +5402,10 @@
 }
 
 int
-dissect_nfs_clientaddr4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_clientaddr4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
-	offset = dissect_nfs_opaque4(tvb, offset, tree, "network id");
-	offset = dissect_nfs_opaque4(tvb, offset, tree, "universal address");
+	offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_data);
+	offset = dissect_nfsdata(tvb, offset, tree, hf_nfs_data);
 
 	return offset;
 }
@@ -5518,12 +5413,11 @@
 
 int
 dissect_nfs_cb_client4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
-	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_cb_program, 
-		offset);
-	offset = dissect_nfs_clientaddr4(tvb, offset, tree, "cb_location");
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_cb_program, offset);
+	offset = dissect_nfs_clientaddr4(tvb, offset, tree);
+
 	return offset;
 }
 
@@ -5552,18 +5446,6 @@
 	return offset;
 }
 
-int
-dissect_nfs_opaque4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
-{
-	/* TODO: use parameter "name" */
-	return dissect_nfsdata(tvb, offset, tree, hf_nfs_data);
-}
-
-/* There is probably a better (built-in?) way to do this, but this works
- * for now.
- */
-
 static const value_string names_nfsv4_operation[] = {
 	{	NFS4_OP_ACCESS,					"ACCESS"	},
 	{	NFS4_OP_CLOSE,						"CLOSE"	},
@@ -5645,9 +5527,8 @@
 
 int
 dissect_nfs_dirlist4(tvbuff_t *tvb, int offset, packet_info *pinfo,
-	proto_tree *tree, char *name _U_)
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	proto_tree *newftree = NULL;
 	guint nextentry;
 
@@ -5663,7 +5544,7 @@
 	{
 		/* offset = dissect_nfs_cookie4(tvb, offset, pinfo, newftree); */
 		offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_cookie4, offset);
-		offset = dissect_nfs_component4(tvb, offset, newftree, "name");
+		offset = dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 		offset = dissect_nfs_fattr4(tvb, offset, pinfo, newftree, "attrs");
 		nextentry = tvb_get_ntohl(tvb, offset);
 		offset += 4;
@@ -5690,8 +5571,10 @@
 		if (newftree) {
 			offset = dissect_rpc_bool(tvb, newftree, 
 				hf_nfs_change_info4_atomic, offset);
-			offset = dissect_nfs_changeid4(tvb, offset, newftree, "before");
-			offset = dissect_nfs_changeid4(tvb, offset, newftree, "after");
+			offset = dissect_rpc_uint64(tvb, tree, hf_nfs_changeid4_before, 
+				offset);
+			offset = dissect_rpc_uint64(tvb, tree, hf_nfs_changeid4_after, 
+				offset);
 		}
 	}
 
@@ -5714,26 +5597,26 @@
 };
 
 int
-dissect_nfs_lock4denied(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_lock4denied(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_offset4, offset);
-	offset = dissect_nfs_length4(tvb, offset, tree, "length");
+	offset = dissect_rpc_uint64(tvb, tree, hf_nfs_length4, offset);
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_lock_type4, offset);
-	offset = dissect_nfs_lock_owner4(tvb, offset, tree, "owner");
+	offset = dissect_nfs_lock_owner4(tvb, offset, tree);
+
 	return offset;
 }
 
 
 int
-dissect_nfs_ace4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_ace4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	offset = dissect_nfs_acetype4(tvb, offset, tree, "type");
-	offset = dissect_nfs_aceflag4(tvb, offset, tree, "flag");
-	offset = dissect_nfs_acemask4(tvb, offset, tree, "access_mask");
-	return dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_ace4, NULL);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_acetype4, offset);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_aceflag4, offset);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_acemask4, offset);
+	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_ace4, NULL);
+
+	return offset;
 }
 
 static const value_string names_open4_result_flags[] = {
@@ -5829,20 +5712,17 @@
 {
 	offset = dissect_nfs_stateid4(tvb, offset, tree);
 	offset = dissect_rpc_bool(tvb, tree, hf_nfs_recall4, offset);
-	offset = dissect_nfs_ace4(tvb, offset, tree, "permissions");
+	offset = dissect_nfs_ace4(tvb, offset, tree);
 
 	return offset;
 }
 
 int
-dissect_nfs_modified_limit4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_modified_limit4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
-	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_num_blocks, 
-		offset);
-	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_bytes_per_block,
-		offset);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_num_blocks, offset);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_bytes_per_block, offset);
+
 	return offset;
 }
 
@@ -5856,9 +5736,8 @@
 
 int
 dissect_nfs_space_limit4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	guint limitby;
 
 	limitby = tvb_get_ntohl(tvb, offset);
@@ -5873,8 +5752,7 @@
 		break;
 
 	case NFS_LIMIT_BLOCKS:
-		offset = dissect_nfs_modified_limit4(tvb, offset, tree, 
-			"mod_blocks");
+		offset = dissect_nfs_modified_limit4(tvb, offset, tree);
 		break;
 
 	default:
@@ -5890,8 +5768,10 @@
 {
 	offset = dissect_nfs_stateid4(tvb, offset, tree);
 	offset = dissect_rpc_bool(tvb, tree, hf_nfs_recall, offset);
-	offset = dissect_nfs_space_limit4(tvb, offset, tree, "space_limit");
-	return dissect_nfs_ace4(tvb, offset, tree, "permissions");
+	offset = dissect_nfs_space_limit4(tvb, offset, tree);
+	offset = dissect_nfs_ace4(tvb, offset, tree);
+
+	return offset;
 }
 
 #define OPEN_DELEGATE_NONE 0
@@ -5905,10 +5785,8 @@
 };
 
 int
-dissect_nfs_open_delegation4(tvbuff_t *tvb, int offset,
-	proto_tree *tree, char *name _U_)
+dissect_nfs_open_delegation4(tvbuff_t *tvb, int offset, proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	guint delegation_type;
 	proto_tree *newftree = NULL;
 	proto_item *fitem = NULL;
@@ -5967,7 +5845,7 @@
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_seqid4, offset);
 	offset = dissect_nfs_stateid4(tvb, offset, tree);
 	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_lock_seqid4, offset);
-	offset = dissect_nfs_lock_owner4(tvb, offset, tree, "owner");
+	offset = dissect_nfs_lock_owner4(tvb, offset, tree);
 
 	return offset;
 }
@@ -6065,14 +5943,13 @@
 				switch(create_type)
 				{
 				case NF4LNK:
-					offset = dissect_nfs_linktext4(tvb, offset, newftree, 
-						"linkdata");
+					offset = dissect_nfs_utf8string(tvb, offset, newftree, 
+						hf_nfs_linktext4, NULL);
 					break;
 				
 				case NF4BLK:
 				case NF4CHR:
-					offset = dissect_nfs_specdata4(tvb, offset,
-						newftree, "devdata");
+					offset = dissect_nfs_specdata4(tvb, offset, newftree);
 					break;
 
 				case NF4SOCK:
@@ -6084,8 +5961,7 @@
 					break;
 				}
 
-				offset = dissect_nfs_component4(tvb, offset, newftree, 
-					"objname");
+				offset = dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 
 				offset = dissect_nfs_fattr4(tvb, offset, pinfo, newftree, 
 					"createattrs");
@@ -6093,8 +5969,7 @@
 			break;
 
 		case NFS4_OP_DELEGPURGE:
-			offset = dissect_rpc_uint64(tvb, newftree, 
-				hf_nfs_clientid4, offset);
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_clientid4, offset);
 			break;
 
 		case NFS4_OP_DELEGRETURN:
@@ -6103,15 +5978,14 @@
 
 		case NFS4_OP_GETATTR:
 			offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, 
-				"attr_request", FATTR4_BITMAP_ONLY);
+				FATTR4_BITMAP_ONLY);
 			break;
 
 		case NFS4_OP_GETFH:
 			break;
 
 		case NFS4_OP_LINK:
-			offset = dissect_nfs_component4(tvb, offset, newftree, 
-				"newname");
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 			break;
 
 		case NFS4_OP_LOCK:
@@ -6121,18 +5995,17 @@
 				offset);
 			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4,
 				offset);
-			offset = dissect_nfs_length4(tvb, offset, newftree, "length");
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_length4, offset);
 			offset = dissect_nfs_locker4(tvb, offset, newftree);
 			break;
 
 		case NFS4_OP_LOCKT:
 			offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_lock_type4,
 				offset);
-			offset = dissect_nfs_lock_owner4(tvb, offset, newftree, 
-				"owner");
+			offset = dissect_nfs_lock_owner4(tvb, offset, newftree);
 			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4,
 				offset);
-			offset = dissect_nfs_length4(tvb, offset, newftree, "length");
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_length4, offset);
 			break;
 
 		case NFS4_OP_LOCKU:
@@ -6142,12 +6015,11 @@
 			offset = dissect_nfs_stateid4(tvb, offset, newftree);
 			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4,
 				offset);
-			offset = dissect_nfs_length4(tvb, offset, newftree, "length");
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_length4, offset);
 			break;
 
 		case NFS4_OP_LOOKUP:
-			offset = dissect_nfs_component4(tvb, offset, newftree, 
-				"objname");
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, hf_nfs_component4, NULL);
 			break;
 
 		case NFS4_OP_LOOKUPP:
@@ -6212,7 +6084,7 @@
 				hf_nfs_count4_dircount, offset);
 			offset = dissect_rpc_uint32(tvb, newftree,
 				hf_nfs_count4_maxcount, offset);
-			offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, "attr", 
+			offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, 
 				FATTR4_BITMAP_ONLY);
 			break;
 
@@ -6220,19 +6092,19 @@
 			break;
 
 		case NFS4_OP_REMOVE:
-			offset = dissect_nfs_component4(tvb, offset, newftree, 
-				"target");
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, 
+				hf_nfs_component4, NULL);
 			break;
 
 		case NFS4_OP_RENAME:
-			offset = dissect_nfs_component4(tvb, offset, newftree, 
-				"oldname");
-			offset = dissect_nfs_component4(tvb, offset, newftree, 
-				"newname");
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, 
+				hf_nfs_component4, NULL);
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, 
+				hf_nfs_component4, NULL);
 			break;
 
 		case NFS4_OP_RENEW:
-			offset = dissect_nfs_clientid4(tvb, offset, newftree);
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_clientid4, offset);
 			break;
 	
 		case NFS4_OP_RESTOREFH:
@@ -6240,7 +6112,8 @@
 			break;
 
 		case NFS4_OP_SECINFO:
-			offset = dissect_nfs_component4(tvb, offset, newftree, "name");
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, 
+				hf_nfs_component4, NULL);
 			break;
 
 		case NFS4_OP_SETATTR:
@@ -6255,15 +6128,14 @@
 
 				fitem = proto_tree_add_text(newftree, tvb, offset, 0, "client");
 
-				if (fitem) {
-					client_tree = proto_item_add_subtree(fitem, 
-						ett_nfs_client_id4);
+				if (fitem) 
+				{
+					client_tree = proto_item_add_subtree(fitem, ett_nfs_client_id4);
 
 					if (newftree)
 					{
-						offset = dissect_nfs_clientid4(tvb, offset,
-							client_tree);
-
+						offset = dissect_rpc_uint64(tvb, ftree, hf_nfs_clientid4, 
+							offset);
 						offset = dissect_nfsdata(tvb, offset, client_tree, 
 							hf_nfs_client_id4_id); 
 					}
@@ -6273,14 +6145,13 @@
 				if (fitem) {
 					newftree = proto_item_add_subtree(fitem, ett_nfs_cb_client4);
 					if (newftree)
-						offset = dissect_nfs_cb_client4(tvb, offset, newftree, 
-							"callback");
+						offset = dissect_nfs_cb_client4(tvb, offset, newftree);
 				}
 			}
 			break;
 
 		case NFS4_OP_SETCLIENTID_CONFIRM:
-			offset = dissect_nfs_clientid4(tvb, offset, newftree);
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_clientid4, offset);
 			break;
 		
 		case NFS4_OP_VERIFY:
@@ -6290,11 +6161,9 @@
 
 		case NFS4_OP_WRITE:
 			offset = dissect_nfs_stateid4(tvb, offset, newftree);
-			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4,
-				offset);
-			offset = dissect_nfs_stable_how4(tvb, offset, newftree, 
-				"stable");
-			offset = dissect_nfs_opaque4(tvb, offset, newftree, "data");
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_offset4, offset);
+			offset = dissect_nfs_stable_how4(tvb, offset, newftree, "stable");
+			offset = dissect_nfsdata(tvb, offset, newftree, hf_nfs_data);
 			break;
 		
 		default:
@@ -6310,18 +6179,47 @@
 	proto_tree* tree)
 {
 	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_tag4, NULL);
-	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_minorversion,
-		offset);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs_minorversion, offset);
 	offset = dissect_nfs_argop4(tvb, offset, pinfo, tree);
 
 	return offset;
 }
 
 int
+dissect_nfs_secinfo4_res(tvbuff_t *tvb, int offset, packet_info *pinfo,
+	proto_tree *tree)
+{
+	guint flavor;
+	proto_item *fitem;
+	proto_tree *secftree;
+				
+	flavor = tvb_get_ntohl(tvb, offset);
+	fitem = proto_tree_add_uint(tree, hf_nfs_secinfo_flavor, tvb, offset, 4, 
+		flavor);
+	offset += 4;
+
+	if (fitem) 
+	{
+		switch(flavor)
+		{
+		case RPCSEC_GSS:
+			secftree = proto_item_add_subtree(fitem, ett_nfs_secinfo4_flavor_info);
+			if (secftree)
+				offset = dissect_nfs_rpcsec_gss_info(tvb, offset, secftree);
+			break;
+
+		default:
+			break;
+		}
+	}
+
+	return offset;
+}
+
+int
 dissect_nfs_resop4(tvbuff_t *tvb, int offset, packet_info *pinfo, 
-	proto_tree *tree, char *name _U_)
+	proto_tree *tree)
 {
-	/* TODO: use parameter "name" */
 	guint ops, ops_counter;
 	guint opcode;
 	proto_item *fitem;
@@ -6330,6 +6228,10 @@
 	guint32 status;
 
 	ops = tvb_get_ntohl(tvb, offset+0);
+
+	if (ops < 0)
+		return offset;		/* malformed result */
+	
 	fitem = proto_tree_add_text(tree, tvb, offset, 4, 
 		"Operations (count: %d)", ops);
 	offset += 4;
@@ -6391,7 +6293,7 @@
 			offset = dissect_nfs_change_info4(tvb, offset, newftree, 
 				"change_info");
 			offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree,
-				"attrsset", FATTR4_BITMAP_ONLY);
+				FATTR4_BITMAP_ONLY);
 			break;
 
 		case NFS4_OP_GETATTR:
@@ -6414,8 +6316,7 @@
 				offset = dissect_nfs_stateid4(tvb, offset, newftree);
 			else
 			if (status == NFS4ERR_DENIED)
-				offset = dissect_nfs_lock4denied(tvb, offset, newftree, 
-					"denied");
+				offset = dissect_nfs_lock4denied(tvb, offset, newftree);
 			break;
 
 		case NFS4_OP_LOCKU:
@@ -6429,9 +6330,8 @@
 			offset = dissect_nfs_open4_rflags(tvb, offset, newftree, 
 				"result_flags");
 			offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, 
-				"attrsset", FATTR4_BITMAP_ONLY);
-			offset = dissect_nfs_open_delegation4(tvb, offset, newftree,
-				"delegation");
+				FATTR4_BITMAP_ONLY);
+			offset = dissect_nfs_open_delegation4(tvb, offset, newftree);
 			break;
 
 		case NFS4_OP_OPEN_CONFIRM:
@@ -6440,19 +6340,18 @@
 			break;
 
 		case NFS4_OP_READ:
-			offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_eof, 
-				offset);
-			offset = dissect_nfs_opaque4(tvb, offset, newftree, "data");
+			offset = dissect_rpc_uint32(tvb, newftree, hf_nfs_eof, offset);
+			offset = dissect_nfsdata(tvb, offset, newftree, hf_nfs_data);
 			break;
 
 		case NFS4_OP_READDIR:
-			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_verifier4, 
-				offset);
-			offset = dissect_nfs_dirlist4(tvb, offset, pinfo, newftree, "reply");
+			offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_verifier4, offset);
+			offset = dissect_nfs_dirlist4(tvb, offset, pinfo, newftree);
 			break;
 
 		case NFS4_OP_READLINK:
-			offset = dissect_nfs_linktext4(tvb, offset, newftree, "link");
+			offset = dissect_nfs_utf8string(tvb, offset, newftree, 
+				hf_nfs_linktext4, NULL);
 			break;
 
 		case NFS4_OP_REMOVE:
@@ -6468,54 +6367,22 @@
 			break;
 
 		case NFS4_OP_SECINFO:
-			{
-				guint data_follows;
-				guint flavor;
-				proto_item *fitem;
-				proto_tree *secftree;
-
-				while ((data_follows = tvb_get_ntohl(tvb, offset)))
-				{
-					offset += 4;
-
-					flavor = tvb_get_ntohl(tvb, offset);
-					fitem = proto_tree_add_uint(tree, hf_nfs_secinfo_flavor, tvb, 
-							offset, 4, flavor);
-					offset += 4;
-
-					if (fitem) 
-					{
-						switch(flavor)
-						{
-							case RPCSEC_GSS:
-								secftree = proto_item_add_subtree(fitem, 
-										ett_nfs_secinfo4_flavor_info);
-								if (secftree)
-									offset = dissect_nfs_rpcsec_gss_info(tvb, offset,
-											secftree);
-								break;
-
-							default:
-								break;
-						}
-					}
-				}
-			}
+			offset = dissect_rpc_list(tvb, pinfo, tree, offset, 
+				dissect_nfs_secinfo4_res);
 			break;
 
 		case NFS4_OP_SETATTR:
 			offset = dissect_nfs_attributes(tvb, offset, pinfo, newftree, 
-				"attrsset", FATTR4_BITMAP_ONLY);
+				FATTR4_BITMAP_ONLY);
 			break;
 
 		case NFS4_OP_SETCLIENTID:
 			if (status == NFS4_OK)
-				offset = dissect_rpc_uint64(tvb, newftree,
-					hf_nfs_clientid4, offset);
+				offset = dissect_rpc_uint64(tvb, newftree, hf_nfs_clientid4, 
+					offset);
 			else
 			if (status == NFS4ERR_CLID_INUSE)
-				offset = dissect_nfs_clientaddr4(tvb, offset, newftree,
-					"client_using");
+				offset = dissect_nfs_clientaddr4(tvb, offset, newftree);
 			break;
 
 		case NFS4_OP_WRITE:
@@ -6543,7 +6410,7 @@
 
 	offset = dissect_nfs_nfsstat4(tvb, offset, tree, &status);
 	offset = dissect_nfs_utf8string(tvb, offset, tree, hf_nfs_tag4, NULL);
-	offset = dissect_nfs_resop4(tvb, offset, pinfo, tree, "arguments");
+	offset = dissect_nfs_resop4(tvb, offset, pinfo, tree);
 
 	return offset;
 }
@@ -7204,6 +7071,14 @@
 		{ &hf_nfs_changeid4, {
 			"changeid", "nfs.changeid4", FT_UINT64, BASE_DEC,
 			NULL, 0, "nfs.changeid4", HFILL }},
+
+		{ &hf_nfs_changeid4_before, {
+			"changeid", "nfs.changeid4.before", FT_UINT64, BASE_DEC,
+			NULL, 0, "nfs.changeid4.before", HFILL }},
+
+		{ &hf_nfs_changeid4_after, {
+			"changeid", "nfs.changeid4.after", FT_UINT64, BASE_DEC,
+			NULL, 0, "nfs.changeid4.after", HFILL }},
 
 		{ &hf_nfs_nfstime4_seconds, {
 			"seconds", "nfs.nfstime4.seconds", FT_UINT64, BASE_DEC,