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
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
