Re: [PATCH v4] module: replace module_layout with module_memory

[Christophe Leroy]

Le 31/01/2023 à 15:06, Peter Zijlstra a écrit :
> On Tue, Jan 31, 2023 at 12:14:48PM +0000, Christophe Leroy wrote:
> 
>>>> Something like
>>>>
>>>> 	return within(addr, mod, MOD_TEXT) || within(addr, mod, MOD_DATA) ||
>>>> 	       within(addr, mod, MOD_RODATA) || within(addr, mod,
>>>> MOD_RO_AFTER_INIT);
>>>
>>> Urgh, how about?
>>>
>>> 	for_each_mod_mem_type(type) {
>>> 		if (!mod_mem_type_is_init(type) && within(addr, mod, type))
>>> 			return true;
>>> 	}
>>> 	return false;
>>>
>>> Then you have have a bunch of mod_mem_type_id_foo() filter functions
>>> that are non-contiguous without having to endlessly repeat stuff
>>> manually.
>>
>> But that's un-readable.
> 
> "For all except init."
> 
>> You have to have the list of possible types in front of you in order to
>> understand what the function does. Which means that one day or another
>> someone will change the order of types in the enum, and it will break.
> 
> I really don't agree, if you do explicit type lists everywhere you have
> to update each and every sites when you modify the enum.
> 
> If you make category helpers, like: data, text, init, then you only need
> to update the helpers without having to worry about each site. Only if
> you add an enum that doesn't fit the existing categories do you need to
> do something new.
> 

Well we misunderstood each other then.

I agree with you that category helpers are worth it.

My point was that the implementation of those category helpers need to 
be explicit and not hide types based on some assumption on their order 
in the enum.