Date: Fri, 22 Aug 2008 11:59:07 +0200 From: Ivan Voras <ivoras@freebsd.org> To: freebsd-arch@freebsd.org Subject: Re: Magic symlinks redux Message-ID: <g8m2lb$r78$1@ger.gmane.org> In-Reply-To: <20080822090448.GB57441@onelab2.iet.unipi.it> References: <g8kv7v$sp2$1@ger.gmane.org> <20080822090448.GB57441@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
Luigi Rizzo wrote: > On Fri, Aug 22, 2008 at 01:54:29AM +0200, Ivan Voras wrote: >> I was reading about new things in NetBSD, and one thing caught my >> attention: per-user /tmp. See >> http://www.feyrer.de/NetBSD/bx/blosxom.cgi/nb_20080714_0251.html for >> example. >> >> Google says that a discussion about magic symlinks happens every now and >> then in FreeBSD but nothing really gets done. I found this >> implementation which looks like it's for 7.0: >> >> http://butcher.heavennet.ru/patches/kernel/magiclinks/ > > interestingly simple. Yes, that's a big part of the attractiveness of this patch :) > Question - is the process' ENV easily available in this part > of the kernel, so one could in principle use environment variables > as replacement strings ? > > Some comments on the code in the above patch: > > + readability it might be improved a bit: > e.g. I don't see why uma_{zalloc|zfree} are hidden behind macros, > nor why symlynk_magic() isn't simply called as > > if (vfs_magiclinks) > symlink_magic(td, cp, &linklen); > > as it cannot fail as implemented; Ok. > also, the whole MATCH/SUBSTITUTE macros are not particularly > readable -- i understand one needs macros to implement sizeof("somestring") > correctly, but apart from a wrapper I believe the core of these two > blocks should be implemented by functions (possibly inline) with > MATCH() returning the match length so one doesn't need to replicate > the string parameter in SUBSTITUTE(); Yes, I intended to remove the code macros into static inline functions, possibly with some macro glue for sizeof. > + correctness -- > 1. termchar is not reset to '/' if a match is not found > 2. what is the intended behaviour when the replacement string overflows > the buffer ? I'll check those later, when I make an updated patch. > + efficiency of symlink_magic() might be improved too: > e.g. the function could do a quick check for the presence of @ and return > without allocation/deallocation if not found; I think it's because the author wanted a single pass over the string (in case of the "extended" @{...} syntax we can't just check if cp[0] == '@'). The first few lines of the symlink_magic loop ("if (cp[i] != '@')") effectively do what strchr() does. > getcredhostname() (and similar routines) could be called so that > they write directly to tmp, without the need for > allocating an in-stack buffer I think this is for consistency in calling SUBSTITUTE. It could be broken into two variants of the code but is it worth it? I'd rather modify getcredhostname() in kern/kern_jail.c to return the length of the created string and avoid a strlen().
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?g8m2lb$r78$1>