Date: Thu, 4 Jan 2007 14:59:17 -0500 From: John Baldwin <jhb@freebsd.org> To: Oliver Fromme <olli@lurza.secnetix.de> Cc: erik.udo@gmail.com, freebsd-hackers@freebsd.org, dougb@freebsd.org Subject: Re: Init.c, making it chroot Message-ID: <200701041459.18321.jhb@freebsd.org> In-Reply-To: <200701041803.l04I3oDo068148@lurza.secnetix.de> References: <200701041803.l04I3oDo068148@lurza.secnetix.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 04 January 2007 13:03, Oliver Fromme wrote:
>
> John Baldwin wrote:
> > Oliver Fromme wrote:
> > > I've created (and tested!) a new patch. I've tested on
> > > RELENG_6, but I think init(8) isn't very different on
> > > HEAD, so it should work there, too.
> > >
> > > Any comments are welcome. I particularly appreciate
> > > if others test this stuff.
> >
> > Some things I noticed:
> >
> > - Why do you have the 'ichroot_name' and 'iscript_name' variables? I
would
> > just pass the string literal to the kenv() function, e.g.
> >
> > if (kenv(KENV_GET, "init_script", kenv_value, sizeof(kenv_value)) > 0) {
> >
> > I think that putting the constant right there is easier for someone who
> > is reading the code to see what is going on.
>
> In fact that's what I tried first ... Alas:
> warning: passing arg 2 of `kenv' discards qualifiers from pointer target
type
It's fixed in HEAD, I'll MFC the prototype fix for kenv() to 6.x.
> > - Rather than abusing a global runcom_script variable that you change to
> > get side effects when you invoke runcom(), why not change runcom() to
> > take a single 'char *script' as an argument and just pass _PATH_RUNCOM
> > or kenv_value as appropriate and get rid of the global runcom_script
> > variable?
>
> You are right, the global runcom_script variable does not
> look very clean. However, the problem is that runcom() is
> one of the transition action functions, i.e. it is called
> by the transition() function and never gets an argument.
>
> Of course it is possible to write an additional function
> run_script(char *script) which contains runcom's current
> code, and make the runcom() function a wrapper that just
> calls run_script(_PATH_RUNCOM). This isn't a perfectly
> clean solution either, but maybe it's at least a little bit
> better.
>
> e.g. basically:
>
> state_func_t
> runcom (void)
> {
> return run_script(_PATH_RUNCOM);
> }
>
> state_func_t
> run_script (char *script)
> {
> /* all the code formerly in runcom() */
> }
>
> Then the init_script code would call run_script(kenv_value)
> instead of runcom(), of course.
>
> Would that be acceptable? Or do you have an even better
> solution in mind?
That sounds great.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200701041459.18321.jhb>