Patchwork [5/8] Use arrays to represent pathnames in struct fedfs_fsl

login
register
mail settings
Submitter Chuck Lever
Date Dec. 21, 2011, 10:52 p.m.
Message ID <20111221225203.23950.77159.stgit@degas.1015granger.net>
Download mbox | patch
Permalink /patch/132753/
State Accepted
Headers show

Comments

Chuck Lever - Dec. 21, 2011, 10:52 p.m.
Since the beginning, I've tried to stick with representing local
pathnames using a C string, and converting to the array form only when
I need to transmit the pathname off-system.  That seemed more natural
for operating on Linux, whose VFS is POSIX-style.

However, if we want to allow creation of junctions that point to
servers that don't necessarily use '/' as the pathname component
separator, we will need a more general way to represent pathnames in
struct fedfs_fsl.

That general way to represent pathnames is as an array of component
character strings.  This is, after all, what is done for fs_locations
in NFS, and is how pathnames are stored and passed in FedFS.

Thus, for example, to allow callers full expression of the FSL data
type, a plug-in API would express pathnames using an array of
components instead of a simple string containing slashes.

Aside from exposing the richer fs_locations data structure to FedFS
callers, this change moves an important part of pathname resolution
policy out of libnsdb and into callers.  It should also provide an
opportunity for stronger and more consistent checks on incoming
pathnames.

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

 src/fedfsd/svc.c             |    2 +-
 src/include/nsdb.h           |    2 +-
 src/libnsdb/administrator.c  |    4 ++--
 src/libnsdb/fileserver.c     |    4 ++--
 src/libnsdb/ldap.c           |   24 +++++++++++++++++-------
 src/libnsdb/nsdb-internal.h  |    2 +-
 src/nsdbc/nsdb-create-fsl.c  |    7 ++++---
 src/nsdbc/nsdb-resolve-fsn.c |    9 ++++++++-
 src/resolve-junction/main.c  |   10 ++++++++--
 9 files changed, 44 insertions(+), 20 deletions(-)

Patch

diff --git a/src/fedfsd/svc.c b/src/fedfsd/svc.c
index 9772df7..60c6fb4 100644
--- a/src/fedfsd/svc.c
+++ b/src/fedfsd/svc.c
@@ -692,7 +692,7 @@  fedfsd_fill_in_fedfsfsl(const struct fedfs_fsl *fsl, FedFsFsl *new)
 		goto out_free;
 	}
 
-	retval = nsdb_posix_to_fedfspathname(fsl->fl_u.fl_nfsfsl.fn_path,
+	retval = nsdb_path_array_to_fedfspathname(fsl->fl_u.fl_nfsfsl.fn_nfspath,
 					&new->FedFsFsl_u.nfsFsl.path);
 	if (retval != FEDFS_OK)
 		goto out_free;
diff --git a/src/include/nsdb.h b/src/include/nsdb.h
index 84cabaf..cbde86f 100644
--- a/src/include/nsdb.h
+++ b/src/include/nsdb.h
@@ -59,7 +59,7 @@  struct fedfs_secdata {
  * the NSDB protocol draft, chapter 4, section 2.2.4
  */
 struct fedfs_nfs_fsl {
-	char			 *fn_path;
+	char			**fn_nfspath;
 	int			  fn_majorver;
 	int			  fn_minorver;
 	int			  fn_currency;
diff --git a/src/libnsdb/administrator.c b/src/libnsdb/administrator.c
index cff679c..23f38d9 100644
--- a/src/libnsdb/administrator.c
+++ b/src/libnsdb/administrator.c
@@ -624,7 +624,7 @@  nsdb_create_nfs_fsl_entry_s(LDAP *ld, const char *nce, struct fedfs_fsl *fsl,
 	sprintf(ttybuf, "%d", fsl->fl_fslttl);
 	nsdb_init_add_attribute(attrs[i++], "fedfsFslTTL", ttyvals, ttybuf);
 
-	retval = nsdb_posix_path_to_xdr(nfsfsl->fn_path, &xdr_path);
+	retval = nsdb_path_array_to_xdr(nfsfsl->fn_nfspath, &xdr_path);
 	if (retval != FEDFS_OK)
 		return retval;
 	xdrpathvals[0] = &xdr_path;
@@ -714,7 +714,7 @@  nsdb_create_nfs_fsl_entry_s(LDAP *ld, const char *nce, struct fedfs_fsl *fsl,
 	retval = FEDFS_OK;
 
 out:
-	free(xdr_path.bv_val);
+	ber_memfree(xdr_path.bv_val);
 	return retval;
 }
 
diff --git a/src/libnsdb/fileserver.c b/src/libnsdb/fileserver.c
index aa3a5b2..3cf3673 100644
--- a/src/libnsdb/fileserver.c
+++ b/src/libnsdb/fileserver.c
@@ -58,7 +58,7 @@  nsdb_free_fedfs_fsl(struct fedfs_fsl *fsl)
 {
 	switch (fsl->fl_type) {
 	case FEDFS_NFS_FSL:
-		free(fsl->fl_u.fl_nfsfsl.fn_path);
+		nsdb_free_string_array(fsl->fl_u.fl_nfsfsl.fn_nfspath);
 		break;
 	default:
 		xlog(L_ERROR, "%s: Unrecognized FSL type", __func__);
@@ -753,7 +753,7 @@  nsdb_resolve_fsn_parse_attribute(LDAP *ld, LDAPMessage *entry, char *attr,
 
 	else if (strcasecmp(attr, "fedfsNfsPath") == 0)
 		retval = nsdb_parse_singlevalue_xdrpath(attr, values,
-					&nfsl->fn_path);
+					&nfsl->fn_nfspath);
 	else if (strcasecmp(attr, "fedfsNfsMajorVer") == 0)
 		retval = nsdb_parse_singlevalue_int(attr, values,
 				&nfsl->fn_majorver);
diff --git a/src/libnsdb/ldap.c b/src/libnsdb/ldap.c
index 5bff920..26331d6 100644
--- a/src/libnsdb/ldap.c
+++ b/src/libnsdb/ldap.c
@@ -309,14 +309,15 @@  nsdb_parse_singlevalue_str(char *attr, struct berval **values,
  *
  * @param attr a NUL-terminated C string containing the name of an attribute
  * @param values pointer to a berval containing value of fedfsNfsPath attribute
- * @param result OUT: dynamically allocated buffer containing XDR-decoded path
+ * @param result OUT: dynamically allocated string array containing XDR-decoded path
  * @return a FedFsStatus code
  *
- * Caller must free "result" with free(3)
+ * Caller must free "result" with nsdb_free_string_array(3)
  */
 FedFsStatus
-nsdb_parse_singlevalue_xdrpath(char *attr, struct berval **values, char **result)
+nsdb_parse_singlevalue_xdrpath(char *attr, struct berval **values, char ***result)
 {
+	char **tmp, *pathname;
 	FedFsStatus retval;
 
 	if (values[1] != NULL) {
@@ -325,15 +326,24 @@  nsdb_parse_singlevalue_xdrpath(char *attr, struct berval **values, char **result
 		return FEDFS_ERR_NSDB_RESPONSE;
 	}
 
-	retval = nsdb_xdr_to_posix_path(values[0], result);
+	retval = nsdb_xdr_to_path_array(values[0], &tmp);
 	if (retval != FEDFS_OK) {
-		xlog(L_ERROR, "%s: Bad %s value",
-			__func__, attr);
+		xlog(L_ERROR, "%s: Bad %s value", __func__, attr);
+		return retval;
+	}
+
+	/* This is useful sanity checking while implementations
+	 * remain immature */
+	retval = nsdb_path_array_to_posix(tmp, &pathname);
+	if (retval != FEDFS_OK) {
+		xlog(L_ERROR, "%s: Reverse conversion failed", __func__);
 		return retval;
 	}
 
 	xlog(D_CALL, "%s: Attribute %s contains value \'%s\'",
-		__func__, attr, *result);
+		__func__, attr, pathname);
+	free(pathname);
+	*result = tmp;
 	return FEDFS_OK;
 }
 
diff --git a/src/libnsdb/nsdb-internal.h b/src/libnsdb/nsdb-internal.h
index b6414b1..969fe48 100644
--- a/src/libnsdb/nsdb-internal.h
+++ b/src/libnsdb/nsdb-internal.h
@@ -71,7 +71,7 @@  FedFsStatus	 nsdb_parse_singlevalue_str(char *attr,
 				struct berval **values, char *result,
 				const size_t len);
 FedFsStatus	 nsdb_parse_singlevalue_xdrpath(char *attr,
-				struct berval **values, char **result);
+				struct berval **values, char ***result);
 FedFsStatus	 nsdb_parse_multivalue_str(char *attr,
 				struct berval **values, char ***result);
 
diff --git a/src/nsdbc/nsdb-create-fsl.c b/src/nsdbc/nsdb-create-fsl.c
index 9dc3c1b..1e2ee59 100644
--- a/src/nsdbc/nsdb-create-fsl.c
+++ b/src/nsdbc/nsdb-create-fsl.c
@@ -215,9 +215,10 @@  main(int argc, char **argv)
 	strcpy(fsl->fl_fsnuuid, fsn_uuid);
 	strcpy(fsl->fl_nsdbname, nsdbname);
 	strcpy(fsl->fl_fslhost, servername);
-	fsl->fl_u.fl_nfsfsl.fn_path = strdup(serverpath);
-	if (fsl->fl_u.fl_nfsfsl.fn_path == NULL) {
-		fprintf(stderr, "Failed to allocate serverpath\n");
+	retval = nsdb_posix_to_path_array(serverpath,
+						&fsl->fl_u.fl_nfsfsl.fn_nfspath);
+	if (retval != FEDFS_OK) {
+		fprintf(stderr, "Failed to encode serverpath\n");
 		goto out;
 	}
 	fsl->fl_nsdbport = nsdbport;
diff --git a/src/nsdbc/nsdb-resolve-fsn.c b/src/nsdbc/nsdb-resolve-fsn.c
index e4d99fc..8a9d5f8 100644
--- a/src/nsdbc/nsdb-resolve-fsn.c
+++ b/src/nsdbc/nsdb-resolve-fsn.c
@@ -108,7 +108,14 @@  _display_bool(const _Bool value)
 static void
 nsdb_resolve_fsn_display_nfs_fsl(struct fedfs_nfs_fsl *nfsl)
 {
-	printf(" NFS fli_rootpath:\t\t%s\n", nfsl->fn_path);
+	FedFsStatus status;
+	char *rootpath;
+
+	status = nsdb_path_array_to_posix(nfsl->fn_nfspath, &rootpath);
+	if (status != FEDFS_OK)
+		return;
+	printf(" NFS fli_rootpath:\t\t%s\n", rootpath);
+	free(rootpath);
 	printf(" NFS major version:\t\t%d\n", nfsl->fn_majorver);
 	printf(" NFS minor version:\t\t%d\n", nfsl->fn_minorver);
 	printf(" NFS fls_currency:\t\t%d\n", nfsl->fn_currency);
diff --git a/src/resolve-junction/main.c b/src/resolve-junction/main.c
index 00f7a1f..601fdd7 100644
--- a/src/resolve-junction/main.c
+++ b/src/resolve-junction/main.c
@@ -188,8 +188,14 @@  _display_bool(const _Bool value)
 static void
 resolve_junction_display_nfs_fsl(struct fedfs_nfs_fsl *nfsl)
 {
-	fprintf(stdout, "fli_rootpath_len: %zu\n", strlen(nfsl->fn_path));
-	fprintf(stdout, "fli_rootpath: %s\n", nfsl->fn_path);
+	char *rootpath;
+
+	if (nsdb_path_array_to_posix(nfsl->fn_nfspath, &rootpath) != FEDFS_OK)
+		return;
+	fprintf(stdout, "fli_rootpath_len: %zu\n", strlen(rootpath));
+	fprintf(stdout, "fli_rootpath: %s\n", rootpath);
+	free(rootpath);
+
 	fprintf(stdout, "major version: %d\n", nfsl->fn_majorver);
 	fprintf(stdout, "minor version: %d\n", nfsl->fn_minorver);
 	fprintf(stdout, "fls_currency: %d\n", nfsl->fn_currency);