On 14.02.2025 21:05, [email protected] wrote:
> --- a/xen/common/version.c
> +++ b/xen/common/version.c
> @@ -210,9 +210,28 @@ void __init xen_build_init(void)
>          }
>      }
>  #endif /* CONFIG_X86 */
> -    if ( !rc )
> -        printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p);
>  }
> +
> +void print_version(void)
> +{
> +    printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n",
> +           xen_major_version(), xen_minor_version(), xen_extra_version(),
> +           xen_compile_by(), xen_compile_domain(), xen_compiler(),
> +           xen_build_info(), xen_compile_date());
> +
> +    printk("Latest ChangeSet: %s\n", xen_changeset());
> +}
> +
> +void print_build_id(void)
> +{
> +    /*
> +     * NB: build_id_p may be NULL if XEN_HAS_BUILD_ID=n.
> +     * Do not print empty build-id.
> +     */
> +    if ( build_id_p )
> +        printk("build-id: %*phN\n", build_id_len, build_id_p);
> +}

In my reply to v3 I specifically suggested to use build_id_len in the if().
Why did you choose to use build_id_p instead? Yes, if all works correctly
both should be (non-)zero/NULL at the same time, but please also consider
the case of things not working correctly. When len is zero, there's nothing
there, no matter what the pointer. When len is non-zero and the pointer is
NULL, it would be quite nice to have a trace thereof in the log.

Preferably with the adjustment (which I'd be happy to make while committing)
Reviewed-by: Jan Beulich <[email protected]>

Jan

Reply via email to