Date: Mon, 15 Jan 2007 09:33:36 +0100 From: Divacky Roman <xdivac02@stud.fit.vutbr.cz> To: Scot Hetzel <swhetzel@gmail.com> Cc: Alexander Leidinger <Alexander@leidinger.net>, freebsd-emulation@freebsd.org Subject: Re: linuxolator: proc/filesystems and sysfs function implementations Message-ID: <20070115083336.GA2073@stud.fit.vutbr.cz> In-Reply-To: <790a9fff0701141254s2c92b10ag6b042a019bc283c@mail.gmail.com> References: <790a9fff0701060012x40063f48pc842510b082df5a5@mail.gmail.com> <20070106130830.3c2e6d98@Magellan.Leidinger.net> <790a9fff0701132017g6c081567la4a759cea4618535@mail.gmail.com> <20070114105239.GA6955@stud.fit.vutbr.cz> <790a9fff0701141254s2c92b10ag6b042a019bc283c@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> >+ 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070115083336.GA2073>