[nfs] Rewrite NFS URI handling

Get the NFS URI manipulation code out of nfs_open.c.  The resulting
code is now much more readable.

Signed-off-by: Marin Hannache <git@mareo.fr>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/include/ipxe/errfile.h

@@ -223,6 +223,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #define ERRFILE_icmp			( ERRFILE_NET | 0x00390000 )
 #define ERRFILE_ping			( ERRFILE_NET | 0x003a0000 )
 #define ERRFILE_dhcpv6			( ERRFILE_NET | 0x003b0000 )
+#define ERRFILE_nfs_uri			( ERRFILE_NET | 0x003c0000 )
 
 #define ERRFILE_image		      ( ERRFILE_IMAGE | 0x00000000 )
 #define ERRFILE_elf		      ( ERRFILE_IMAGE | 0x00010000 )

src/include/ipxe/nfs_uri.h

@@ -0,0 +1,29 @@
+#ifndef _IPXE_NFS_URI_H
+#define _IPXE_NFS_URI_H
+
+/** @file
+ *
+ * Network File System protocol URI handling functions
+ *
+ */
+
+FILE_LICENCE ( GPL2_OR_LATER );
+
+#include <ipxe/uri.h>
+
+struct nfs_uri {
+	char *mountpoint;
+	char *filename;
+	char *path;
+	char *lookup_pos;
+};
+
+int nfs_uri_init ( struct nfs_uri *nfs_uri, const struct uri *uri );
+int nfs_uri_next_mountpoint ( struct nfs_uri *uri );
+int nfs_uri_symlink ( struct nfs_uri *uri, const char *symlink_value );
+char *nfs_uri_mountpoint ( const struct nfs_uri *uri );
+char *nfs_uri_next_path_component ( struct nfs_uri *uri );
+void nfs_uri_free ( struct nfs_uri *uri );
+
+
+#endif /* _IPXE_NFS_URI_H */

src/net/oncrpc/nfs_open.c

@@ -40,6 +40,7 @@
 #include <ipxe/oncrpc_iob.h>
 #include <ipxe/portmap.h>
 #include <ipxe/mount.h>
+#include <ipxe/nfs_uri.h>
 
 /** @file
  *
@@ -101,10 +102,7 @@ struct nfs_request {
 	struct oncrpc_cred_sys  auth_sys;
 
 	char *                  hostname;
-	char *                  path;
-	char *                  mountpoint;
-	char *                  filename;
-	size_t                  filename_offset;
+	struct nfs_uri          uri;
 
 	struct nfs_fh           readlink_fh;
 	struct nfs_fh           current_fh;
@@ -127,8 +125,9 @@ static void nfs_free ( struct refcnt *refcnt ) {
 	nfs = container_of ( refcnt, struct nfs_request, refcnt );
 	DBGC ( nfs, "NFS_OPEN %p freed\n", nfs );
 
+	nfs_uri_free ( &nfs->uri );
+
 	free ( nfs->hostname );
-	free ( nfs->path );
 	free ( nfs->auth_sys.hostname );
 	free ( nfs );
 }
@@ -145,8 +144,6 @@ static void nfs_done ( struct nfs_request *nfs, int rc ) {
 
 	DBGC ( nfs, "NFS_OPEN %p completed (%s)\n", nfs, strerror ( rc ) );
 
-	free ( nfs->filename - nfs->filename_offset );
-
 	intf_shutdown ( &nfs->xfer, rc );
 	intf_shutdown ( &nfs->pm_intf, rc );
 	intf_shutdown ( &nfs->mount_intf, rc );
@@ -276,10 +273,10 @@ static void nfs_mount_step ( struct nfs_request *nfs ) {
 
 	if ( nfs->mount_state == NFS_MOUNT_NONE ) {
 		DBGC ( nfs, "NFS_OPEN %p MNT call (%s)\n", nfs,
-		       nfs->mountpoint );
+		       nfs_uri_mountpoint ( &nfs->uri ) );
 
 		rc = mount_mnt ( &nfs->mount_intf, &nfs->mount_session,
-		                 nfs->mountpoint );
+		                 nfs_uri_mountpoint ( &nfs->uri ) );
 		if ( rc != 0 )
 			goto err;
 
@@ -291,7 +288,7 @@ static void nfs_mount_step ( struct nfs_request *nfs ) {
 		DBGC ( nfs, "NFS_OPEN %p UMNT call\n", nfs );
 
 		rc = mount_umnt ( &nfs->mount_intf, &nfs->mount_session,
-		                  nfs->mountpoint );
+		                 nfs_uri_mountpoint ( &nfs->uri ) );
 		if ( rc != 0 )
 			goto err;
 	}
@@ -305,7 +302,6 @@ static int nfs_mount_deliver ( struct nfs_request *nfs,
                                struct io_buffer *io_buf,
                                struct xfer_metadata *meta __unused ) {
 	int                     rc;
-	char                    *sep;
 	struct oncrpc_reply     reply;
 	struct mount_mnt_reply  mnt_reply;
 
@@ -320,30 +316,30 @@ static int nfs_mount_deliver ( struct nfs_request *nfs,
 		DBGC ( nfs, "NFS_OPEN %p got MNT reply\n", nfs );
 		rc = mount_get_mnt_reply ( &mnt_reply, &reply );
 		if ( rc != 0 ) {
-			if ( mnt_reply.status != MNT3ERR_NOTDIR )
+			switch ( mnt_reply.status ) {
+				case MNT3ERR_NOTDIR:
+				case MNT3ERR_NOENT:
+				case MNT3ERR_ACCES:
+					break;
+
+				default:
+					goto err;
+			}
+
+			if ( ! strcmp ( nfs_uri_mountpoint ( &nfs->uri ),
+				        "/" ) )
 				goto err;
 
-			if ( strcmp ( nfs->mountpoint, "/" ) == 0 )
+			if ( ( rc = nfs_uri_next_mountpoint ( &nfs->uri ) ) )
 				goto err;
 
-			sep = strrchr ( nfs->mountpoint, '/' );
-			nfs->filename[-1]    = '/';
-			nfs->filename_offset = sep + 1 - nfs->filename;
-			nfs->filename        = sep + 1;
-			*sep = '\0';
+			DBGC ( nfs, "NFS_OPEN %p MNT failed retrying with " \
+			       "%s\n", nfs, nfs_uri_mountpoint ( &nfs->uri ) );
 
-			DBGC ( nfs, "NFS_OPEN %p ENOTDIR received retrying" \
-			       "with %s\n", nfs, nfs->mountpoint );
-			goto done;
-		}
+			nfs->mount_state--;
+			nfs_mount_step ( nfs );
 
-		/* We need to strdup() nfs->filename since the code handling
-		 * symlink resolution make the assumption that it can be
-		 * free()ed. */
-		if ( ( nfs->filename = strdup ( nfs->filename ) ) == NULL )
-		{
-			rc = -ENOMEM;
-			goto err;
+			goto done;
 		}
 
 		nfs->current_fh = mnt_reply.fh;
@@ -370,22 +366,13 @@ done:
 
 static void nfs_step ( struct nfs_request *nfs ) {
 	int     rc;
-	char    *path_component = NULL;
+	char    *path_component;
 
 	if ( ! xfer_window ( &nfs->nfs_intf ) )
 		return;
 
 	if ( nfs->nfs_state == NFS_LOOKUP ) {
-		while ( path_component == NULL || path_component[0] == '\0') {
-			path_component = nfs->filename;
-			while ( *nfs->filename != '\0' ) {
-				nfs->filename_offset++;
-				if ( *nfs->filename++ == '/' ) {
-					*(nfs->filename - 1) = '\0';
-					break;
-				}
-			}
-		}
+		path_component = nfs_uri_next_path_component ( &nfs->uri );
 
 		DBGC ( nfs, "NFS_OPEN %p LOOKUP call (%s)\n", nfs,
                        path_component );
@@ -455,11 +442,11 @@ static int nfs_deliver ( struct nfs_request *nfs,
 
 		if ( lookup_reply.ent_type == NFS_ATTR_SYMLINK ) {
 			nfs->readlink_fh = lookup_reply.fh;
-			nfs->nfs_state = NFS_READLINK;
+			nfs->nfs_state   = NFS_READLINK;
 		} else {
 			nfs->current_fh = lookup_reply.fh;
 
-			if ( nfs->filename[0] == '\0' )
+			if ( nfs->uri.lookup_pos[0] == '\0' )
 				nfs->nfs_state = NFS_READ;
 			else
 				nfs->nfs_state--;
@@ -470,7 +457,7 @@ static int nfs_deliver ( struct nfs_request *nfs,
 	}
 
 	if ( nfs->nfs_state == NFS_READLINK_SENT ) {
-		char                      *new_filename;
+		char                      *path;
 		struct nfs_readlink_reply readlink_reply;
 
 		DBGC ( nfs, "NFS_OPEN %p got READLINK reply\n", nfs );
@@ -480,46 +467,23 @@ static int nfs_deliver ( struct nfs_request *nfs,
 			goto err;
 
 		if ( readlink_reply.path_len == 0 )
-			return -EINVAL;
-
-		if ( readlink_reply.path[0] == '/' ) {
-			if ( strncmp ( readlink_reply.path, nfs->mountpoint,
-			               strlen ( nfs->mountpoint ) ) != 0 )
-				return -EINVAL;
-
-			/* The mountpoint part of the path is ended by a '/' */
-			if ( strlen ( nfs->mountpoint ) !=
-			     readlink_reply.path_len ) {
-				readlink_reply.path     += 1;
-				readlink_reply.path_len -= 1;
-			}
-
-			/* We are considering the last part of the absolute
-			 * path as a relative path from the mountpoint.
-			 */
-			readlink_reply.path     += strlen ( nfs->mountpoint );
-			readlink_reply.path_len -= strlen ( nfs->mountpoint );
+		{
+			rc = -EINVAL;
+			goto err;
 		}
 
-		new_filename = malloc ( readlink_reply.path_len +
-		                        strlen ( nfs->filename ) + 2 );
-		if ( ! new_filename ) {
+		if ( ! ( path = strndup ( readlink_reply.path,
+		                          readlink_reply.path_len ) ) )
+		{
 			rc = -ENOMEM;
 			goto err;
 		}
 
-		memcpy ( new_filename, readlink_reply.path,
-		         readlink_reply.path_len );
-		strcpy ( new_filename + readlink_reply.path_len + 1,
-		         nfs->filename );
-		new_filename[readlink_reply.path_len] = '/';
-
-		free ( nfs->filename - nfs->filename_offset );
-		nfs->filename = new_filename;
-		nfs->filename_offset = 0;
+		nfs_uri_symlink ( &nfs->uri, path );
+		free ( path );
 
-		DBGC ( nfs, "NFS_OPEN %p new filename: %s\n", nfs,
-		       nfs->filename );
+		DBGC ( nfs, "NFS_OPEN %p new path: %s\n", nfs,
+		       nfs->uri.path );
 
 		nfs->nfs_state = NFS_LOOKUP;
 		nfs_step ( nfs );
@@ -638,30 +602,21 @@ static int nfs_parse_uri ( struct nfs_request *nfs, const struct uri *uri ) {
 	if ( ! uri || ! uri->host || ! uri->path )
 		return -EINVAL;
 
-	if ( ! ( nfs->path = strdup ( uri->path ) ) )
-		return -ENOMEM;
+	if ( ( rc = nfs_uri_init ( &nfs->uri, uri ) ) != 0 )
+		return rc;
 
 	if ( ! ( nfs->hostname = strdup ( uri->host ) ) ) {
 		rc = -ENOMEM;
 		goto err_hostname;
 	}
 
-	nfs->filename   = basename ( nfs->path );
-	nfs->mountpoint = dirname ( nfs->path );
-
-	if ( nfs->filename[0] == '\0' )
-		goto err_filename;
-
-	DBGC ( nfs, "NFS_OPEN %p URI parsed: (mountpoint=%s, filename=%s)\n",
-	       nfs, nfs->mountpoint, nfs->filename );
+	DBGC ( nfs, "NFS_OPEN %p URI parsed: (mountpoint=%s, path=%s)\n",
+	       nfs, nfs_uri_mountpoint ( &nfs->uri), nfs->uri.path );
 
 	return 0;
 
-err_filename:
-	rc = -EINVAL;
-	free ( nfs->hostname );
 err_hostname:
-	free ( nfs->path );
+	nfs_uri_free ( &nfs->uri );
 	return rc;
 }
 
@@ -698,6 +653,9 @@ static int nfs_open ( struct interface *xfer, struct uri *uri ) {
 	mount_init_session ( &nfs->mount_session, &nfs->auth_sys.credential );
 	nfs_init_session ( &nfs->nfs_session, &nfs->auth_sys.credential );
 
+	DBGC ( nfs, "NFS_OPEN %p connecting to port mapper (%s:%d)...\n", nfs,
+	       nfs->hostname, PORTMAP_PORT );
+
 	rc = nfs_connect ( &nfs->pm_intf, PORTMAP_PORT, nfs->hostname );
 	if ( rc != 0 )
 		goto err_connect;
@@ -711,6 +669,8 @@ static int nfs_open ( struct interface *xfer, struct uri *uri ) {
 err_connect:
 	free ( nfs->auth_sys.hostname );
 err_cred:
+	nfs_uri_free ( &nfs->uri );
+	free ( nfs->hostname );
 err_uri:
 	free ( nfs );
 	return rc;

src/net/oncrpc/nfs_uri.c

@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2014 Marin Hannache <ipxe@mareo.fr>.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <libgen.h>
+#include <ipxe/nfs_uri.h>
+
+/** @file
+ *
+ * Network File System protocol URI handling functions
+ *
+ */
+
+int nfs_uri_init ( struct nfs_uri *nfs_uri, const struct uri *uri ) {
+	if ( ! ( nfs_uri->mountpoint = strdup ( uri->path ) ) )
+		return -ENOMEM;
+
+	nfs_uri->filename = basename ( nfs_uri->mountpoint );
+	if ( strchr ( uri->path, '/' ) != NULL )
+		nfs_uri->mountpoint = dirname ( nfs_uri->mountpoint );
+
+	if ( nfs_uri->filename[0] == '\0' ) {
+		free ( nfs_uri->mountpoint );
+		return -EINVAL;
+	}
+
+	if ( ! ( nfs_uri->path = strdup ( nfs_uri->filename ) ) ) {
+		free ( nfs_uri->mountpoint );
+		return -ENOMEM;
+	}
+	nfs_uri->lookup_pos = nfs_uri->path;
+
+	return 0;
+}
+
+char *nfs_uri_mountpoint ( const struct nfs_uri *uri ) {
+	if ( uri->mountpoint + 1 == uri->filename ||
+	     uri->mountpoint     == uri->filename )
+		return "/";
+
+	return uri->mountpoint;
+}
+
+int nfs_uri_next_mountpoint ( struct nfs_uri *uri ) {
+	char *sep;
+
+	if ( uri->mountpoint + 1 == uri->filename ||
+	     uri->mountpoint     == uri->filename )
+		return -ENOENT;
+
+	sep = strrchr ( uri->mountpoint, '/' );
+	uri->filename[-1] = '/';
+	uri->filename     = sep + 1;
+	*sep = '\0';
+
+	free ( uri->path );
+	if ( ! ( uri->path = strdup ( uri->filename ) ) ) {
+		uri->path = NULL;
+		return -ENOMEM;
+	}
+	uri->lookup_pos = uri->path;
+
+	return 0;
+}
+
+int nfs_uri_symlink ( struct nfs_uri *uri, const char *symlink ) {
+	size_t len;
+	char *new_path;
+
+	if ( ! uri->path )
+		return -EINVAL;
+
+	if ( *symlink == '/' )
+	{
+		if ( strncmp ( symlink, uri->mountpoint,
+			       strlen ( uri->mountpoint ) ) != 0 )
+			return -EINVAL;
+
+		len = strlen ( uri->lookup_pos ) + strlen ( symlink ) - \
+		      strlen ( uri->mountpoint );
+		if ( ! ( new_path = malloc ( len * sizeof ( char ) ) ) )
+			return -ENOMEM;
+
+		strcpy ( new_path, symlink + strlen ( uri->mountpoint ) );
+		strcpy ( new_path + strlen ( new_path ), uri->lookup_pos );
+
+	} else {
+		len = strlen ( uri->lookup_pos ) + strlen ( symlink );
+		if ( ! ( new_path = malloc ( len * sizeof ( char ) ) ) )
+			return -ENOMEM;
+
+
+		strcpy ( new_path, symlink );
+		strcpy ( new_path + strlen ( new_path ), uri->lookup_pos );
+	}
+
+	free ( uri->path );
+	uri->lookup_pos = uri->path = new_path;
+
+	return 0;
+}
+
+char *nfs_uri_next_path_component ( struct nfs_uri *uri ) {
+	char *sep;
+	char *start;
+
+	if ( ! uri->path )
+		return NULL;
+
+	for ( sep = uri->lookup_pos ; *sep != '\0' && *sep != '/'; sep++ )
+		;
+
+	start = uri->lookup_pos;
+	uri->lookup_pos = sep;
+	if ( *sep != '\0' ) {
+		uri->lookup_pos++;
+		*sep = '\0';
+		if ( *start == '\0' )
+			return nfs_uri_next_path_component ( uri );
+	}
+
+	return start;
+}
+
+void nfs_uri_free ( struct nfs_uri *uri ) {
+	free ( uri->mountpoint );
+	free ( uri->path );
+	uri->mountpoint = NULL;
+	uri->path       = NULL;
+}