On Thu, May 25, 2023 at 10:36 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
>
> The variable "j" should be defined as enum mod_mem_type.
hmm, so can we change the enums to macros for now?
Here, the enums are more flexible than macros, once the kernel changes the enums, crash-utility is easy to follow up kernel changes.
#define MOD_TEXT 0
#define MOD_DATA 1
...
I don't see enum's good point here in crash.
If the value is out of the enums ranges, it may have a warning from the compiler, just a guess, not confirmed.
>> @@ -1808,6 +1911,24 @@ static const char *module_end_tags[] = {
>> "_MODULE_INIT_DATA_END_",
>> "_MODULE_INIT_RODATA_END_"
>> };
>> +static const char *module_start_strs[] = {
>> + "MODULE TEXT START",
>> + "MODULE DATA START",
>> + "MODULE RODATA START",
>> + "MODULE RO_AFTER_INIT START",
>> + "MODULE INIT_TEXT START",
>> + "MODULE INIT_DATA START",
>> + "MODULE INIT_RODATA START"
>> +};
>> +static const char *module_end_strs[] = {
>> + "MODULE TEXT END",
>> + "MODULE DATA END",
>> + "MODULE RODATA END",
>> + "MODULE RO_AFTER_INIT END",
>> + "MODULE INIT_TEXT END",
>> + "MODULE INIT_DATA END",
>> + "MODULE INIT_RODATA END"
>> +};
>>
>>
> As I mentioned in the [PATCH 01/15], similarly the above strings are in
> pairs, so they can be defined as one array or macro substitution.
Yes, how about this?
Also looks good to me. Thanks.
struct module_tag {
char *start;
char *end;
char *start_str;
char *end_str;
};
#define MODULE_TAG(type, suffix) ("_MODULE_" #type "_" #suffix)
#define MODULE_STR(type, suffix) ( "MODULE " #type " " #suffix)
#define MODULE_TAGS(type) { \
.start = MODULE_TAG(type, START), \
.end = MODULE_TAG(type, END), \
.start_str = MODULE_STR(type, START), \
.end_str = MODULE_STR(type, END) \
}
static const struct module_tag module_tag[] = {
MODULE_TAGS(TEXT),
MODULE_TAGS(DATA),
MODULE_TAGS(RODATA),
MODULE_TAGS(RO_AFTER_INIT),
MODULE_TAGS(INIT_TEXT),
MODULE_TAGS(INIT_DATA),
MODULE_TAGS(INIT_RODATA),
};
...
>
>
>> + if (is_insmod_builtin(lm, sp)) {
>> + spnext = sp+1;
>> + if ((spnext < sp_end) &&
>> + (value ==
>> spnext->value))
>> + sp = spnext;
>> + }
>> + if (sp->name[0] == '.') {
>> + spnext = sp+1;
>> + if (spnext->value == value)
>> + sp = spnext;
>> + }
>> + if (offset)
>> + *offset = 0;
>> + return sp;
>> + }
>> +
>> + if (sp->value > value) {
>> + sp = splast ? splast : sp - 1;
>> + if (offset)
>> + *offset = value -
>> sp->value;
>> + return sp;
>> + }
>> +
>>
>
> Why is it needed? The splast will also store the "__insmod_"-type symbols?
To return the previous sp and offset, when value is lower than the
current sp.
Thank you for the explanation, Kazu.
Lianbo
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
