[LEDE-DEV] [PATCH mountd] filesystem mount options in uci config

[John Crispin]

Hi,

comment inline.

On 10/06/2016 11:18, olivier.hardouin at gmail.com wrote:
> Move (previously hardcoded) mount option to UCI to allow different configuration 
> like charset (utf-8 or iso) and filesystem driver (if alternative ones are used).
> The fs names are changed in lowercase to comply with UCI general naming.
> 
> Signed-off-by: Olivier Hardouin <olivier.hardouin at gmail.com>
> ---
>  mount.c | 69 ++++++++++++++++++++++++++---------------------------------------
>  1 file changed, 28 insertions(+), 41 deletions(-)
> 
> diff --git a/mount.c b/mount.c
> index 8892040..c8f7ea6 100644
> --- a/mount.c
> +++ b/mount.c
> @@ -51,15 +51,15 @@ struct mount {
>  char *fs_names[] = {
>  	"",
>  	"",
> -	"MBR",
> -	"EXT2",
> -	"EXT3",
> -	"FAT",
> -	"HFSPLUS",
> +	"mbr",
> +	"ext2",
> +	"ext3",
> +	"fat",
> +	"hfsplus",
>  	"",
> -	"NTFS",
> +	"ntfs",
>  	"",
> -	"EXT4"
> +	"ext4"
>  };
>  
>  #define MAX_MOUNTED		32
> @@ -227,42 +227,29 @@ int mount_new(char *path, char *dev)
>  	pid = autofs_safe_fork();
>  	if(!pid)
>  	{
> -		if(mount->fs == EXFAT)
> +		if(mount->fs > MBR && mount->fs <= EXT4)
>  		{
> -			log_printf("mount -t exfat -o rw,uid=1000,gid=1000 /dev/%s %s", mount->dev, tmp);
> -			ret = system_printf("mount -t exfat -o rw,uid=1000,gid=1000 /dev/%s %s", mount->dev, tmp);
> -		}
> -		if(mount->fs == FAT)
> -		{
> -			log_printf("mount -t vfat -o rw,uid=1000,gid=1000 /dev/%s %s", mount->dev, tmp);
> -			ret = system_printf("mount -t vfat -o rw,uid=1000,gid=1000 /dev/%s %s", mount->dev, tmp);
> -		}
> -		if(mount->fs == EXT4)
> -		{
> -			log_printf("mount -t ext4 -o rw,defaults /dev/%s %s", mount->dev, tmp);
> -			ret = system_printf("mount -t ext4 -o rw,defaults /dev/%s %s", mount->dev, tmp);
> -		}
> -		if(mount->fs == EXT3)
> -		{
> -			log_printf("mount -t ext3 -o rw,defaults /dev/%s %s", mount->dev, tmp);
> -			ret = system_printf("mount -t ext3 -o rw,defaults /dev/%s %s", mount->dev, tmp);
> -		}
> -		if(mount->fs == EXT2)
> -		{
> -			log_printf("mount -t ext2 -o rw,defaults /dev/%s %s", mount->dev, tmp);
> -			ret = system_printf("mount -t ext2 -o rw,defaults /dev/%s %s", mount->dev, tmp);
> -		}
> -		if(mount->fs == HFSPLUS)
> -		{
> -			log_printf("mount -t hfsplus -o rw,defaults,uid=1000,gid=1000 /dev/%s %s", mount->dev, tmp);
> -			ret = system_printf("mount -t hfsplus -o rw,defaults,uid=1000,gid=1000 /dev/%s %s", mount->dev, tmp);
> -		}
> -		if(mount->fs == NTFS)
> -		{
> -			log_printf("ntfs-3g /dev/%s %s -o force", mount->dev, tmp);
> -			ret = system_printf("ntfs-3g /dev/%s %s -o force", mount->dev, tmp);
> +			struct uci_context *ctx;
> +			char *options, *fstype;
> +			ctx = ucix_init("mountd");
> +			options = ucix_get_option(ctx, "mountd", fs_names[mount->fs], "options");
> +			fstype = ucix_get_option(ctx, "mountd", fs_names[mount->fs], "fstype");

this might be NULL but is used below without checking for NULL. i am not
sure these even needs to be loaded from uci as it is the same as
fs_names[mount->fs].

could you look into this and then resend the patch ?

	John

> +			ucix_cleanup(ctx);
> +			if(!fstype)
> +			{
> +				log_printf("mounting /dev/%s failed, expecting 'fstype' uci parameter (kernel driver) for %s", mount->dev, fs_names[mount->fs]);
> +			} else {
> +				if(!options)
> +				{
> +					log_printf("mount -t %s /dev/%s %s", fstype, mount->dev, tmp);
> +					ret = system_printf("mount -t %s /dev/%s %s", fstype, mount->dev, tmp);
> +				} else {
> +					log_printf("mount -t %s -o %s /dev/%s %s", fstype, options, mount->dev, tmp);
> +					ret = system_printf("mount -t %s -o %s /dev/%s %s", fstype, options, mount->dev, tmp);
> +				}
> +			}
> +			exit(WEXITSTATUS(ret));
>  		}
> -		exit(WEXITSTATUS(ret));
>  	}
>  	pid = waitpid(pid, &ret, 0);
>  	ret = WEXITSTATUS(ret);
>