Hi,
0xBADC0DE wrote:
This is my current code:
Code:
// Get memory address of loaded GRUB module
unsigned int module_address = mbinfo->mods_addr;
multiboot_module_t modules[mbinfo->mods_count];
// Stored the start and end address of the module in the struct
for (unsigned int i=0; i<mbinfo->mods_count; i++) {
modules[i].mod_start = module_address + (i*4);
modules[i].mod_end = module_address + ((i+1) * 4);
modules[i].cmdline = module_address + ((i+2) * 4);
modules[i].pad = module_address + ((i+3) * 4);
}
printf("Modules[0] start addr=%d\nModules[0] end addr= %d\nsizeof(modules)=%d\n", modules[0].mod_start, modules[0].mod_end, sizeof(modules));
printf("Modules[0] char=%c\n", modules[0].mod_start);
"mbinfo->mods_count" gives "1"
"mbinfo->mods_addr" gives "166464"
As you can see, I am trying to create an array of structs containing each module's start and end address. I'm looping through each module, as you suggested, and trying to store the information in the struct, adding an offset to "module_address".
There's multiple issues here.
First, there's a problem with the maths - e.g. "module_address + ((i+3) * 4)" looks like it was supposed to be "module_address + ((i*4) + 3)".
Second, there's a problem with pointer dereferencing - e.g. if "module_address + (i*4);" correctly calculates the address of the data you want, then "modules[i].mod_start = module_address + (i*4);" will store the address in "modules[i].mod_start" and won't store the data you want in "modules[i].mod_start".
Third, you're not using the language's type system effectively. For example, if you used "void * module_address" (instead of "unsigned int module_address") the compiler will know that the variable "module_address" contains the address of something and not just an integer value, and if you used "multiboot_module_t * module_address" the compiler would know that the variable "module_address" contains the address of one or more "multiboot_module_t" structures.
Fourth, if GRUB says there's no modules your code will copy nothing and then assume that it copied at least one, and will then blow up.
Fifth, GRUB already gave you a list of "multiboot_module_t" structures, so why are you copying them to create your own list of "multiboot_module_t" structures? Assuming virtual memory is the same as physical memory (e.g. everything identity mapped or paging not enabled, and segmentation not used); your code could delete all the buggy copying and just use "multiboot_module_t * modules = (multiboot_module_t *) mbinfo->mods_addr;".
Sixth, your code to print stuff has the similar bugs - using the address of some data where you wanted the data and not the address. For example, "printf("Modules[0] char=%c\n", modules[0].mod_start);" will convert the 32-bit physical address of the module into a character (mostly likely discarding most of it so it fits in 8 bits) and won't read a byte/character from the memory at that address.
Based on all of these bugs (combined with being unable to recognise some of them yourself), I'd be tempted to suggest that you might benefit from gaining more experience with C (especially pointers) before you start writing an OS in C.
Cheers,
Brendan