Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> +enum {
> + SHFL_FN_QUERY_MAPPINGS = 1, /* Query mappings changes. */
> + SHFL_FN_QUERY_MAP_NAME, /* Query map name. */
> + SHFL_FN_CREATE, /* Open/create object. */
> + SHFL_FN_CLOSE, /* Close object handle. */
> + SHFL_FN_READ, /* Read object content. */
> + SHFL_FN_WRITE, /* Write new object content. */
> + SHFL_FN_LOCK, /* Lock/unlock a range in the object. */
> + SHFL_FN_LIST, /* List object content. */
> + SHFL_FN_INFORMATION, /* Query/set object information. */
> + /* Note function number 10 is not used! */
> + SHFL_FN_REMOVE = 11, /* Remove object */
> + SHFL_FN_MAP_FOLDER_OLD, /* Map folder (legacy) */
> + SHFL_FN_UNMAP_FOLDER, /* Unmap folder */
> + SHFL_FN_RENAME, /* Rename object */
> + SHFL_FN_FLUSH, /* Flush file */
> + SHFL_FN_SET_UTF8, /* Select UTF8 filename encoding */
> + SHFL_FN_MAP_FOLDER, /* Map folder */
> + SHFL_FN_READLINK, /* Read symlink dest (as of VBox 4.0) */
> + SHFL_FN_SYMLINK, /* Create symlink (as of VBox 4.0) */
> + SHFL_FN_SET_SYMLINKS, /* Ask host to show symlinks (as of 4.0) */
> +};
If these are protocol numbers that can't be changed, I would assign the value
on all of them. If they're used by userspace, should they be moved into a
uapi header (and the same for the other stuff in this file)?
> +static const struct fs_parameter_spec vboxsf_param_specs[] = {
> + fsparam_string("nls", opt_nls),
> + fsparam_u32("uid", opt_uid),
> + fsparam_u32("gid", opt_gid),
> + fsparam_u32("ttl", opt_ttl),
> + fsparam_u32oct("dmode", opt_dmode),
> + fsparam_u32oct("fmode", opt_fmode),
> + fsparam_u32oct("dmask", opt_dmask),
> + fsparam_u32oct("fmask", opt_fmask),
> + {}
> +};
I would format this with tabs so that everything nicely lines up:
static const struct fs_parameter_spec vboxsf_param_specs[] = {
fsparam_string ("nls", opt_nls),
fsparam_u32 ("uid", opt_uid),
fsparam_u32 ("gid", opt_gid),
fsparam_u32 ("ttl", opt_ttl),
fsparam_u32oct ("dmode", opt_dmode),
fsparam_u32oct ("fmode", opt_fmode),
fsparam_u32oct ("dmask", opt_dmask),
fsparam_u32oct ("fmask", opt_fmask),
{}
};
but, otherwise, good!
> + case opt_uid:
> + ctx->o.uid = result.uint_32;
> + break;
> + case opt_gid:
> + ctx->o.gid = result.uint_32;
> + break;
Should you be using kuid/kgid transforms for the appropriate namespace?
opts->fs_uid = make_kuid(current_user_ns(), option);
if (!uid_valid(opts->fs_uid))
return -EINVAL;
sort of thing (excerpt from fs/fat/inode.c).
> + case opt_ttl:
> + ctx->o.ttl = msecs_to_jiffies(result.uint_32);
> + break;
Is 0 valid?
> + case opt_dmode:
> + ctx->o.dmode = result.uint_32;
> + break;
> + case opt_fmode:
> + ctx->o.fmode = result.uint_32;
> + break;
> + case opt_dmask:
> + ctx->o.dmask = result.uint_32;
> + break;
> + case opt_fmask:
> + ctx->o.fmask = result.uint_32;
> + break;
Do these need vetting? I guess you kind of do:
inode->i_mode =
sf_g->o.dmode != ~0 ? (sf_g->o.dmode & 0777) : mode;
inode->i_mode &= ~sf_g->o.dmask;
I'm guessing you're stuck with the mount options?
> +struct vboxsf_options {
> + int ttl;
jiffies are unsigned long.
> + int uid;
> + int gid;
kuid_t & kgid_t.
> + int dmode;
> + int fmode;
> + int dmask;
> + int fmask;
> +};
umode_t.
