[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] tools/xl: don't crash on NULL command line
On 2025-07-28 06:45, Andrew Cooper wrote: On 28/07/2025 11:24 am, 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?Hmm, I didn't intend to change things, but I also didn't anticipate libxl__strdup()'s behaviour, or for something to depend on that. I think it isn't strdup()'s behaviour, but rather the old code: - xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline); - info->commandline = libxl__strdup(NOGC, u.xen_commandline); + info->commandline = xc_xenver_commandline(ctx->xch);No error checking on xc_version(), so strdup() is duplicating whatever (stale?) data may be in the union. Regards, Jason While this does turn out to be a marginal API change, I think your solution is the right one. I do not think it's reasonable for there to be one magic pointer that has differing NULL-ness to the others, and NULL for "no information" is the better interface.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |