On 6/17/25 11:47 AM, Daniel Gomez wrote:
>> Do you mean the following, or something else:
>>
>> static int move_module(struct module *mod, struct load_info *info)
>> {
>> int i;
>> enum mod_mem_type t = MOD_MEM_NUM_TYPES;
>> int ret;
>> bool codetag_section_found = false;
>>
>> for_each_mod_mem_type(type) {
>> if (!mod->mem[type].size) {
>> mod->mem[type].base = NULL;
>> continue;
>> }
>>
>> ret = module_memory_alloc(mod, type);
>> if (ret) {
>> t = type;
>> goto out_err;
>> }
>> }
>>
>> [...]
>> }
>>
>
> Yes, that's it. From your patch, moving MOD_MEM_NUM_TYPE assigment to the
> initialization and use the while() loop suggested later on.
Ok.
>
> One thing though, we are "releasing" the memory even if we have skipped the
> allocation in the first place. So, I think it would make sense to release only
> for the types we have actually allocated. What do you think?
I noticed this too, specifically because move_module() is inconsistent
in this regard with free_mod_mem(). The latter function contains:
if (mod_mem->size)
module_memory_free(mod, type);
However, my preference is actually to update free_mod_mem() and remove
the check. The function module_memory_free() should be a no-op if
mod->base is NULL, similarly to how calling free(NULL) is a no-op.
--
Thanks,
Petr
