On Sun, Jul 14, 2013 at 02:24:52PM -0400, Kevin O'Connor wrote:
On Sun, Jul 07, 2013 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
Add ability for a ROM file to point to it's image in memory. When file is in memory, add utility that can patch it, storing pointers to one file within another file.
Thanks. See my comments below.
[...]
--- /dev/null +++ b/src/linker.c
[...]
+void linker_loader_execute(const char *name) +{
- struct linker_loader_entry_s *entry;
- int size, offset = 0;
- void *data = romfile_loadfile(name, &size);
- if (!data)
return;
- for (offset = 0; offset < size; offset += sizeof *entry) {
For consistent style, please treat sizeof like a function (ie, sizeof(*entry) ).
entry = data + offset;
/* Check that entry fits in buffer. */
if (offset + sizeof *entry > size) {
warn_internalerror();
break;
}
- switch (le32_to_cpu(entry->command)) {
case LINKER_LOADER_COMMAND_ALLOCATE:
linker_loader_allocate(entry);
SeaBIOS uses 4 spaces for indentation, and no tabs.
[...]
--- a/src/util.h +++ b/src/util.h @@ -436,6 +436,7 @@ struct romfile_s { char name[128]; u32 size; int (*copy)(struct romfile_s *file, void *dest, u32 maxlen);
- void *data;
};
I'd prefer to see this tracked within the "linker" code and not in the generic romfile struct.
A way to associate a romfile instance with a value seems generally useful, no? Still, that's not too hard - it would only mean an extra linked list of
struct linker { char name[56] void *data; struct hlist_node node; }
is this preferable?
Also, is there another name besides "linker" that could be used? SeaBIOS has code to self-relocate and fixup code relocations. I think having code in the repo called "linker" could cause confusion.
-Kevin
romfile_loader?