On 14.01.2023 00:08, Andrew Cooper wrote:
> struct xen_build_id and struct xen_varbuf are identical from an ABI point of
> view, so XENVER_build_id can reuse xenver_varbuf_op() rather than having it's
> own almost identical copy of the logic.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
albeit with a style related question plus remark:
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -476,9 +476,22 @@ static long xenver_varbuf_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> struct xen_varbuf user_str;
> const char *str = NULL;
> size_t sz;
> + int rc;
Why is this declared here, yet ...
> switch ( cmd )
> {
> + case XENVER_build_id:
> + {
> + unsigned int local_sz;
... this declared here? Both could live in switch()'s scope, allowing
for re-use in other case blocks, but making clear that the values are
unavailable outside of the switch().
> + rc = xen_build_id((const void **)&str, &local_sz);
> + if ( rc )
> + return rc;
> +
> + sz = local_sz;
> + goto have_len;
Personally I certainly dislike "goto" in general, and I thought the
common grounds were to permit its use in error handling (only).
Jan
