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