linuxolator: proc/filesystems and sysfs function implementations

[Divacky Roman]

> >+                       buf = (char *)(uintptr_t)args->arg2;
> :
> >+                       bsd_to_linux_fs(vfsp->vfc_name, &fs);
> >+                       len = strlen(fs.name) + 1;
> >+                       error = copyout(fs.name, buf, len);
> >
> >no need to check if it fits in the userspace buffer? also you
> >dont check return value
> >
> 
> According to the linux man page, there isn't a way to check the size
> of buf, as we are only passed the value of the pointer in args->arg2.
> Should I replace buf with  (char *)(uintptr_t)args->arg2 in the
> copyout, since it is only used there.
> 
> http://www.die.net/doc/linux/man/man2/sysfs.2.html
> 
> I didn't check the return value, as error is going to be set to either
> 0 or EFAULT by the copyout function.  Since it's the end of the case,
> it falls thru to the return statement at the bottom of the function.

ok
 
> >+                       break;
> >+
> >+               /*
> >+                * Return the total number of file system types currently 
> >present
> >+                * in the kernel.
> >+                */
> >+               case 3:
> >+                       TAILQ_FOREACH(vfsp, &vfsconf, vfc_list)
> >+                               index++;
> >+                       td->td_retval[0] = index;
> >
> >does this make sense? shouldnt we return just the number of filesystems
> >that we share with linux?
> >
> 
> There are only 7 filesystems that are freebsd specific (devfs,
> fdescfs, mfs, nullfs, portalfs, procfs (bsd), unionfs).  If we exclude
> these filesystems, then we would need to ensure that we skip them in
> the other two options.
> 
> The way that it is currently emulated, is that all freebsd filesystems
> (kldloaded filesystems) are available to the linuxolator.
 
sounds sane... but what happens when userspace program gets FSs it doesnt
know or something? I mean.. is it safe to reveal everything?
 
> >+       else L_NODEV("rootfs")
> >+       else L_NODEV("smbfs")
> >+       else L_NODEV("linsysfs")
> >+       else L_NODEV("unionfs")
> >+       else
> >+               fs->fs_flags = 1;       /* FS_REQUIRES_DEV */
> >+
> >+       F2L_NAME("ext2fs", "ext2")
> >+       else F2L_NAME("cd9660", "iso9660")
> >+       else F2L_NAME("msdosfs", "msdos")
> >+       else F2L_NAME("procfs", "bsdprocfs")
> >+       else F2L_NAME("linprocfs", "proc")
> >+       else F2L_NAME("linsysfs", "sysfs")
> >+       else F2L_NAME("ffs", "ufs")
> >+       else
> >+               strcpy(fs->name, name);
> >+#undef L_NODEV
> >+#undef F2L_NAME
> >
> >man, this is ugly :) cant you come up with some translation table or 
> >something?
> >
> I had created a table (see attached linux.fs), but wasn't sure how to
> parse it, or how to keep it up todate as more filesystems are added.
 
well. this is not performance sensitive so basically any kind of parsing
should do it. I like the table MUCH more then the macro thing you presented
previously. dont worry about performance of the parsing - table with 30 (or so)
entries is not performance critical
 
> >also... you strcpy string to another string, are you sure it always fits? 
> >how do you
> >asure that?
> >
> 
> name can't be larger than MFSNAMELEN, and sizeof(fs->name) == MFSNAMELEN.

sounds fair

> >RCS file: /home/ncvs/src/sys/compat/linux/linux_util.h,v
> >retrieving revision 1.28
> >diff -u -r1.28 linux_util.h
> >--- compat/linux/linux_util.h   27 Jun 2006 18:30:49 -0000      1.28
> >+++ compat/linux/linux_util.h   14 Jan 2007 02:08:42 -0000
> >@@ -101,4 +101,12 @@
> > char   *linux_get_char_devices(void);
> > void   linux_free_get_char_devices(char *string);
> >
> >+struct linux_file_system_type {
> >+       char    name[16];
> >+       int     fs_flags;
> >+};
> >
> >16? why 16? please comment or something
> >
> 16 is the value of MFSNAMELEN and used by the vfsconf struct to define
> the max size of vfc_name.  I kept name (in the linux_file_system_type
> struct) the same size, so that we didn't have to worry about the size
> when using strcpy.
> 
> I also wasn't sure if I should add:
> 
> #include <sys/mount.h>
> 
> in linux_util.h so that MFSNAMELEN could be used to define the size of
> name in the struct (it may also affect other files, that include
> linux_util.h).

definitely use the define MFSNAMELEN instead of 16