>> /* Prepare a memory region to store two memory maps. */ >> memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES >> (MEMORY_MAP_SIZE)); >> if (! memory_map) >> - grub_fatal ("cannot allocate memory"); >> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory"); > > Something more descriptive would be nice, may be even "cannot allocate > memory for memory map"
Yes. >> >> /* Obtain descriptors for available memory. */ >> map_size = MEMORY_MAP_SIZE; >> @@ -588,14 +588,14 @@ grub_efi_mm_init (void) >> >> memory_map = grub_efi_allocate_any_pages (2 * BYTES_TO_PAGES >> (map_size)); >> if (! memory_map) >> - grub_fatal ("cannot allocate memory"); >> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory"); > > Ditto. And maybe nice to have it slightly different, perhaps "cannot > re-allocate memory map" Yes. >> >> mm_status = grub_efi_get_memory_map (&map_size, memory_map, 0, >> &desc_size, 0); >> } >> >> if (mm_status < 0) >> - grub_fatal ("cannot get memory map"); >> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot get memory map"); > > What are the range of values that mm_status could be? Would it be > useful to include the status code in the error message? IOW, could a > failure here be affected by a configuration that the user could change > to make this work? Also I like the message "failed to retrieve memory > map (status=%d)". > The function says: /* Get the memory map as defined in the EFI spec. Return 1 if successful, return 0 if partial, or return -1 if an error occurs. */ So no, I don't think we could print anything more interesting without reworking the grub_efi_get_memory_map function. >> >> memory_map_end = NEXT_MEMORY_DESCRIPTOR (memory_map, map_size); >> >> @@ -610,7 +610,7 @@ grub_efi_mm_init (void) >> >> /* Allocate memory regions for GRUB's memory management. */ >> add_memory_regions (filtered_memory_map, desc_size, >> - filtered_memory_map_end, BYTES_TO_PAGES >> (DEFAULT_HEAP_SIZE)); >> + filtered_memory_map_end, BYTES_TO_PAGES (required_bytes)); >> >> #if 0 >> /* For debug. */ > > What about turning this on when MM_DEBUG is on? Seems like it could be > a useful (would been to get rid of/change the grub_fatal calls). > Perhaps, but I'm not sufficiently across EFI to be confident making that sort of change, and I'm not well set up to test EFI systems or evaluate the correctness or usefulness of any output. I think this would be better deferred to another patch from someone with more experience in the field. Kind regards, Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel