Re: [PATCH] kbuild: Parameterize kallsyms generation and correct reporting

[Masahiro Yamada]

On Tue, Aug 13, 2019 at 6:49 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> When kallsyms generation happens, temporary vmlinux outputs are linked
> but the quiet make output doesn't report it, giving the impression that
> the prior command is taking longer than expected.
>
> Instead, report the KSYM step before the temporary linking. While at it,
> this consolidates the repeated "kallsyms generation step" into a single
> function and removes the existing copy/pasting.
>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---

Hmm, I did not notice this.

How about showing the link stage explicitly?
(Is it too verbose?)

  MODINFO modules.builtin.modinfo
  LD      .tmp_vmlinux1
  KSYMS   .tmp_kallsyms1.o
  LD      .tmp_vmlinux2
  KSYMS   .tmp_kallsyms2.o
  LD      vmlinux
  SORTEX  vmlinux


If this verbosity is OK,
you can move 'info LD  ${2}' into vmlinux_link()




Anyway, I like the clean-ups in this patch.

This is just my personal preference, but
may I ask two cosmetic changes?

[1] Could you move kallsyms_step()
    between  kallsyms() and mksysmap() ?
    I want to collect function definitions
    to the top of the script.

[2] Could you shorten 'kallsymso_previous'
    to 'kallsymso_prev' ?




-- 
Best Regards
Masahiro Yamada