Re: [PATCH] tools/xl: don't crash on NULL command line

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jul 28, 2025 at 12:24:03PM +0200, Marek Marczykowski-Górecki wrote:
> When running xl in a domU, it doesn't have access to the Xen command
> line. Before the non-truncating xc_xenver_cmdline(), it was always set
> with strdup, possibly of an empty string. Now it's NULL. Treat it the
> same as empty cmdline, as it was before. Autoballoon isn't relevant for
> xl devd in a domU anyway.
> 
> Fixes: 75f91607621c ("tools: Introduce a non-truncating xc_xenver_cmdline()")
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> So, apparently the "No API/ABI change" was a lie... it changed "empty
> string" to NULL in libxl_version_info->commandline. Quick search didn't
> spot any other (in-tree) place that could trip on NULL there. IMO NULL
> value in this case makes more sense. Buf maybe for the API stability
> reasons the old behavior should be restored?
> 
> PS I'm working on a CI test for this case (and driver domains in
> general). I have it working with Alpine already, but it wouldn't detect
> this issue, as musl's regexec() doesn't crash on NULL... So, I'll add a
> test on Debian too.
> ---
>  tools/xl/xl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/xl/xl.c b/tools/xl/xl.c
> index ec72ca60c32a..e183d42b1d65 100644
> --- a/tools/xl/xl.c
> +++ b/tools/xl/xl.c
> @@ -81,6 +81,8 @@ static int auto_autoballoon(void)
>      info = libxl_get_version_info(ctx);
>      if (!info)
>          return 1; /* default to on */
> +    if (!info->commandline)
> +        return 1;

It's a nit, but could you join with the previous if condition, so that
the comment also applies?

     if (!info || !info->commandline)
         return 1; /* default to on */

Thanks, Roger.