[patch][linux-cifs-client] add fake mount option (-f) to mount.cifs

[Jeff Layton]

On Mon, 2 Feb 2009 13:33:37 -0600
Shirish Pargaonkar <shirishpargaonkar at gmail.com> wrote:

Looks like the second patch is a superset of the first. I'm assuming
the first isn't actually needed...


> --- client.orig/umount.cifs.c	2009-02-02 04:35:20.000000000 -0600
> +++ client/umount.cifs.c	2009-02-02 05:29:11.000000000 -0600
> @@ -33,6 +33,7 @@
>  #include <errno.h>
>  #include <string.h>
>  #include <mntent.h>
> +#include <limits.h>
>  #include "mount.h"
>  
>  #define UNMOUNT_CIFS_VERSION_MAJOR "0"
> @@ -231,6 +232,37 @@ static int remove_from_mtab(char * mount
>  	return rc;
>  }
>  
> +/* Make a canonical pathname from PATH.  Returns a freshly malloced string.
> +   It is up the *caller* to ensure that the PATH is sensible.  i.e.
> +   canonicalize ("/dev/fd0/.") returns "/dev/fd0" even though ``/dev/fd0/.''
> +   is not a legal pathname for ``/dev/fd0''  Anything we cannot parse
> +   we return unmodified.   */
> +static char *
> +canonicalize (char *path)

Minor style nit ^^^

Kernel coding standards frown on adding spaces between symbol names and
the open parenthesis. This is userspace, but it seems to me that we
ought to follow kernel coding style where possible. Cleaner code is
easier to maintain, and it's easier to attract new developers when the
coding style is more recognizable.

> +{
> +	char *canonical = malloc (PATH_MAX + 1);
> +
> +	if (!canonical) {
> +		fprintf(stderr, "Error! Not enough memory!\n");
> +		return NULL;
> +	}
> +
> +	if (strlen(path) > PATH_MAX) {
> +		fprintf(stderr, "Mount point string too long\n");
> +		return NULL;
> +	}
> +
> +	if (path == NULL)
> +		return NULL;
> +
> +	if (realpath (path, canonical))
> +		return canonical;
> +
> +	strncpy (canonical, path, PATH_MAX);
> +	canonical[PATH_MAX] = '\0';
> +	return canonical;
> +}
> +
>  int main(int argc, char ** argv)
>  {
>  	int c;
> @@ -304,7 +336,7 @@ int main(int argc, char ** argv)
>  	argv += optind;
>  	argc -= optind;
>  
> -	mountpoint = argv[0];
> +	mountpoint = canonicalize(argv[0]);
>  
>  	if((argc < 1) || (argv[0] == NULL)) {
>  		printf("\nMissing name of unmount directory\n");

Other than the minor style nit, this looks fine to me.

I'm wonder though whether we need a umount.cifs program at all. All the
teardown is done in-kernel. Is there some reason we can't just rely on
the normal util-linux-ng umount program?

Cheers,
-- 
Jeff Layton <jlayton at redhat.com>