On Tue, Mar 15, 2022 at 03:21:12PM +0530, Maninder Singh wrote:
> print module information when KALLSYMS is disabled.
>
> No change for %pB, as it needs to know symbol name to adjust address
> value which can't be done without KALLSYMS.
...
> +int sprint_kallsym_common(char *buffer, unsigned long address, int build_id,
> + int backtrace, int symbol)
> +{
> + if (backtrace)
> + return __sprint_symbol(buffer, address, -1, 1, build_id);
> + else if (symbol)
> + return __sprint_symbol(buffer, address, 0, 1, build_id);
> + else
> + return __sprint_symbol(buffer, address, 0, 0, 0);
Redundant 'else' in both cases.
> +}
...
> +static int sprint_module_info(char *buf, char *end, unsigned long value,
> + const char *fmt, int modbuildid, int backtrace, int symbol)
fmt is not used.
> +{
> + struct module *mod;
> + unsigned long offset = 0;
> + unsigned long base;
Can it be the same type as core_layout.base? Why not?
> + char *modname;
> + int len;
> + const unsigned char *buildid = NULL;
> +
> + if (is_ksym_addr(value))
> + return 0;
> +
> + if (backtrace || symbol)
> + offset = 1;
I would expect here to have
else
offset = 0;
But see below.
> + preempt_disable();
> + mod = __module_address(value);
> + if (mod) {
> + modname = mod->name;
> +#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
> + if (modbuildid)
> + buildid = mod->build_id;
> +#endif
> + if (offset) {
This seems quite confusing because semantically you use offset as a boolean
flag and offset. Why not add a boolean variable with a clear name?
> + base = (unsigned long)mod->core_layout.base;
> + offset = value - base;
> + }
> + }
> +
Probably you can drop this blank line to group entire critical section,
or add a blank line above.
> + preempt_enable();
> + if (!mod)
> + return 0;
> +
> + /* address belongs to module */
> + if (offset)
> + len = sprintf(buf, "0x%p+0x%lx", (void *)base, offset);
> + else
> + len = sprintf(buf, "0x%lx", (void *)value);
What this casting is for? Don't you have a compilation warning?
> + len += fill_name_build_id(buf, modname, modbuildid, buildid, len);
> + return len;
return len + ...;
?
> +}
--
With Best Regards,
Andy Shevchenko
