On 13/06/2016 09:47, Olivier Hardouin wrote:
> Hi John,
>
> Thanks for your comment.
> You're right, I need to add some check before getting uci options.
>
> Regarding the fs name, I propose to use per default the same driver name
> as defined in fs_names (and not have the fstype parameter in uci).
> I would still keep the fstype in uci as optional parameter in case an
> alternative driver has to be used (e.g. paragon for ntfs).
>
Hi,
> What do you think about it?
make it optional in that case please. i would prefer to not list ext4
int he ext4 section for it to work. if the fs is not explicitly defined
use the sane default
John
>
> Olivier
>
>
> On Mon, Jun 13, 2016 at 7:03 AM, John Crispin <j...@phrozen.org
> <mailto:j...@phrozen.org>> wrote:
>
> Hi,
>
> comment inline.
>
> On 10/06/2016 11:18, olivier.hardo...@gmail.com
> <mailto:olivier.hardo...@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.hardo...@gmail.com
> <mailto:olivier.hardo...@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);
> >
>
>
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
