[LinuxBIOS] patch to lib/lar.c and include/lar.h

[ron minnich]

These changes are needed to allow initram to chain to subsequent
stage1 phases, per the earlier discussion.

The chaining is needed since initram is going to call disable_car,
which may, among other things, move the stack around
or even lose the contents of stack. Support a return from disable_car
is a very difficult proposition.

The intent is that I can do this:
ret = execute_in_place(&archive, "normal/initram/segment0", main_ram_working);
or similar and thus chain from initram to the second part of main,
with a new stack and working dram.

ron

[Peter Stuge]

On Sat, Dec 15, 2007 at 06:00:43PM -0800, ron minnich wrote:
> Support a return from disable_car is a very difficult proposition.

Aye.
> The intent is that I can do this:
> ret = execute_in_place(&archive, "normal/initram/segment0", main_ram_working);
> or similar and thus chain from initram to the second part of main,
> with a new stack and working dram.

I don't like the patch as is.

It is now becoming non-obvious from function names and parameter
names what the code flow is.

This is a problem in v2 that I would like us to be careful to not
introduce also in v3.
> run_address is passed the address of a function taking no parameters and
> run_address is passed the address of a function taking a single void * parameter
> jumps to it, returning the result.
> @param v the address to call as a function.
> returns value returned by the function.
> */
>
> -int run_address(void *f)
> +int run_address(void *f, void *param)

Please fix up the doxygen comments completely. Note that all
parameters in the doxygen comment are incorrect.
> @@ -263,9 +263,10 @@
> Given a file name in the LAR , search for it, and execute it in place.
> @param archive A descriptor for current archive.
> @param filename filename to find
> @param param void * parameter
> returns 0 on success, -1 otherwise
> */
>
> -int execute_in_place(const struct mem_file *archive, const char *filename)
> +int execute_in_place(const struct mem_file *archive, const char *filename, void *param)

The name 'param' is really not transparent and the doxygen comment is
not clear enough about how this param will be used - ie. that it is
just passed on as a parameter to the function that actually executes
the file.

Generally speaking I think this arrangement is pretty ugly.
(Passing a parameter several layers into the code.)

In this instance I do think it's a pretty good solution, because it
is actually fairly simple, especially considering the alternatives.

I guess I am proposing that we make the parameter names and possibly
also some function names less generic, because this is something that
will be used exclusively during one specific phase transition.

Good or bad?

//Peter

[ron minnich]

why not model the run_address on execv?
This is execv:
int execv(const char *path, char *const argv[]);

So run_address become, instead:
int run_address(void (*f)(void *argv), char *const argv[], void *stack);

both argv and stack can be NULL.

This leaves us with a great deal of flexibility and an interface that
looks like a threat startup.

ron

[ron minnich]

okay peter how's this?

ron