On Thu, Sep 18, 2014 at 09:28:41AM +0100, [email protected] wrote:
> Revised diffbelow:
You are inheriting some bugs in here by reusing newfs code. :)
I agree with the comments from others. Additional comments:
> #include <util.h>
> +#include <paths.h>
...
> + args.ta_root_mode = modeset ? mode : sb.st_mode;
> +
> + int newflags = mntflags;
...
> + if(template) newflags &= ~MNT_RDONLY;
...
> + if(template) {
KNF
> +#include "pathnames.h"
This was too much copy/paste from newfs. You aren't using any of
the definitions in here.
> + if (execv(cmd, argv) != 0)
> + err(1, "%s", cmd);
execv() does not return when successful. On error, it returns -1.
> + strlcpy(mountpoint, src, sizeof(mountpoint));
The strlcpy call should check for truncation. There's no guarantee
that src is less than sizeof(mountpoint) since src = template = optarg.
> + tmp = getenv("TMPDIR");
> + if (tmp == NULL || *tmp == '\0')
> + tmp = _PATH_TMP;
It's being used with mkdtemp(), but I think you should still check
issetugid() before trusting getenv().
+ if (mntflags & MNT_RDONLY) {
+ mntflags |= MNT_UPDATE;
+ if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) < 0)
{
This doesn't look right. /usr/src/sys/tmpfs/tmpfs_vfsops.c doesn't
support MNT_UPDATE in tmpfs_mount().
I don't see a reason for canon_dev to exist. It gets written to but isn't
used anywhere else.
