On Sat, Feb 27, 2016 at 05:44:49PM -0500, Michael McConville wrote:
> * There wasn't yet a list of possible errors for tmpfs in mount(2).
> That said, I question the value of maintaining these lists. It's
> almost impossible for them to be comprehensive - for example, some
> *_mount() functions return the error value of library functions like
> copyin(9) and copyout(9) if they fail. Errors like ENOMEM are
> probably sufficiently self-explanatory to be excluded. These lists
> likely make sense for explaining anomalous errors, though.
I think maintaining the error list is a worthy effort. It's not only
about explaining what the error codes mean, but also giving the
application developer a chance to know which error codes _can_ be
returned by a function in order to be able to handle them correctly. I
don't see why mount(2) should be an exception.
> * The KASSERT() on sys/tmpfs/tmpfs_vfsops.c:160 checks root against
> NULL after calling tmpfs_alloc_node(). However, tmpfs_alloc_node()
> only sets root on success, and in that case the value will always be
> non-null. We should therefore initialize root to NULL. We should
> maybe consider just returning tmpfs_alloc_node()'s error code instead
> of KASSERTing.
Initializing root to NULL is fine with me.
I don't think the KASSERT() should be converted to a return, because it
checks a condition that is not supposed to happen, except in the
presence of programming error in the kernel. Let's have a look at the
possible return values of tmpfs_alloc_node():
[ENOSPC] tmpfs_node_get() returned NULL, which can only
happen when a pool_get with PR_WAITOK does the
same, which must not happen.
[ENOSPC] inode number overflow; must not happen on a
fresh filesystem
IMHO, calling panic() there is the right thing to do.
natano
> Index: lib/libc/sys/mount.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/mount.2,v
> retrieving revision 1.45
> diff -u -p -r1.45 mount.2
> --- lib/libc/sys/mount.2 23 Nov 2015 10:01:45 -0000 1.45
> +++ lib/libc/sys/mount.2 27 Feb 2016 22:21:51 -0000
> @@ -384,6 +384,18 @@ An I/O error occurred while writing cach
> .Fa dir
> points outside the process's allocated address space.
> .El
> +.Pp
> +The following errors can occur for a
> +.Em tmpfs
> +filesystem mount:
> +.Bl -tag -width [ENOMEM]
> +.It Bq Er ENOMEM
> +There is not enough memory available to allocate the mount structures.
> +.It Bq Er ENOTSUPP
> +The
> +.Dv MNT_UPDATE
> +flag, which is not yet supported, was set.
> +.El
> .Sh SEE ALSO
> .Xr statfs 2 ,
> .Xr mount 8 ,
> Index: sys/tmpfs/tmpfs_vfsops.c
> ===================================================================
> RCS file: /cvs/src/sys/tmpfs/tmpfs_vfsops.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 tmpfs_vfsops.c
> --- sys/tmpfs/tmpfs_vfsops.c 13 Jan 2016 13:01:40 -0000 1.8
> +++ sys/tmpfs/tmpfs_vfsops.c 27 Feb 2016 22:21:55 -0000
> @@ -87,7 +87,7 @@ tmpfs_mount(struct mount *mp, const char
> {
> struct tmpfs_args args;
> tmpfs_mount_t *tmp;
> - tmpfs_node_t *root;
> + tmpfs_node_t *root = NULL;
> uint64_t memlimit;
> uint64_t nodes;
> int error;
> @@ -120,7 +120,7 @@ tmpfs_mount(struct mount *mp, const char
>
> /* Prohibit mounts if there is not enough memory. */
> if (tmpfs_mem_info(1) < TMPFS_PAGES_RESERVED)
> - return EINVAL;
> + return ENOMEM;
>
> error = copyin(data, &args, sizeof(struct tmpfs_args));
> if (error)
> @@ -180,9 +180,12 @@ tmpfs_mount(struct mount *mp, const char
>
> mp->mnt_stat.mount_info.tmpfs_args = args;
>
> - bzero(&mp->mnt_stat.f_mntonname, sizeof(mp->mnt_stat.f_mntonname));
> - bzero(&mp->mnt_stat.f_mntfromname, sizeof(mp->mnt_stat.f_mntfromname));
> - bzero(&mp->mnt_stat.f_mntfromspec, sizeof(mp->mnt_stat.f_mntfromspec));
> + memset(&mp->mnt_stat.f_mntonname, 0,
> + sizeof(mp->mnt_stat.f_mntonname));
> + memset(&mp->mnt_stat.f_mntfromname, 0,
> + sizeof(mp->mnt_stat.f_mntfromname));
> + memset(&mp->mnt_stat.f_mntfromspec, 0,
> + sizeof(mp->mnt_stat.f_mntfromspec));
>
> strlcpy(mp->mnt_stat.f_mntonname, path,
> sizeof(mp->mnt_stat.f_mntonname) - 1);
>
