From owner-freebsd-arch@FreeBSD.ORG Fri Aug 22 09:59:17 2008 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8B2BB1065676 for ; Fri, 22 Aug 2008 09:59:17 +0000 (UTC) (envelope-from freebsd-arch@m.gmane.org) Received: from ciao.gmane.org (main.gmane.org [80.91.229.2]) by mx1.freebsd.org (Postfix) with ESMTP id 107988FC3A for ; Fri, 22 Aug 2008 09:59:17 +0000 (UTC) (envelope-from freebsd-arch@m.gmane.org) Received: from list by ciao.gmane.org with local (Exim 4.43) id 1KWTQJ-0004Ao-PJ for freebsd-arch@freebsd.org; Fri, 22 Aug 2008 09:59:15 +0000 Received: from lara.cc.fer.hr ([161.53.72.113]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Fri, 22 Aug 2008 09:59:15 +0000 Received: from ivoras by lara.cc.fer.hr with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Fri, 22 Aug 2008 09:59:15 +0000 X-Injected-Via-Gmane: http://gmane.org/ To: freebsd-arch@freebsd.org From: Ivan Voras Date: Fri, 22 Aug 2008 11:59:07 +0200 Lines: 68 Message-ID: References: <20080822090448.GB57441@onelab2.iet.unipi.it> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: lara.cc.fer.hr User-Agent: Thunderbird 2.0.0.16 (X11/20080724) In-Reply-To: <20080822090448.GB57441@onelab2.iet.unipi.it> Sender: news Subject: Re: Magic symlinks redux X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Aug 2008 09:59:17 -0000 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().