[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v2] xen/hypercalls: address violations of MISRA C:2012 Rule 8.3


  • To: Federico Serafini <federico.serafini@xxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 23 Aug 2023 08:28:32 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=vYeb64JCwWvXk7Z2bH5oaqf4AGb0MMwXWawtvf0cB9E=; b=dTrXIwIOA05njw0xdV6aDlNfVQd3+TlEYwEY4tIy7LP9UhLokJ9+gP5G8UtIm/+DUqeflM4ifPOVVagGIy5Ur+hfx6JW2K9JU80aygavj0cv1S8sUTYZE1NyF8d4jZK7phveSBCbcfP806KTjUGKZlZfYuVT6sRbnWOIWIj77ej491zFLotNu3i+Ruswe2IMy18RZq76lxxjbjHr84KkTBdX8fBEJr0iRtDqgfQ7qhempe71k+3avrmpfynM7Dri56T/H2Q4jpC4ltJ63MZf4E5DOJ1oo7WJBNi7QFruia33IHIML7eYS6oRKuao80ck8dVIXSfNHpGQG9m+uWfPPQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FL0bGzHCaMV92QIUTbMcOLi4DJ2wHMJeqWaWm2mTytZH+RvhnuE7eGOxUt+QRKXbDa9vwpyNfYUQ13QGY9aQ0vL/sAv7kc0uvvA7EbQOqecFil83ikeRYsB2f4jZQoVWpS4Xz1l1uGpYVN7HprOYDY+xURacoUU2LhEDJqN1oj4kQaGQwpCut5JYLimFIzFaPTdYZ6BKskL+VV8TaM5K1OT4n1DG8pZn//9qpB+q5o0ThnSNoPV/PDtRz9splqvcjOkLO6275o+Vnm4ft+B6TYePzjmYSEYVZTz/zve/DCur35LD/C9yyKryPa61JrWT7UhDZ+UenQBlvFbZSBFHOA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: consulting@xxxxxxxxxxx, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Wed, 23 Aug 2023 07:28:57 +0000
  • Ironport-data: A9a23:9/4aCKLzvKghtI8vFE+RU5QlxSXFcZb7ZxGr2PjKsXjdYENS0DYDz zEeC2rQOfuDY2f2fNoia9u190kDu8fdytdjHVRlqX01Q3x08seUXt7xwmUcnc+xBpaaEB84t ZV2hv3odp1coqr0/0/1WlTZhSAgk/rOHvykU7Ss1hlZHWdMUD0mhQ9oh9k3i4tphcnRKw6Ws Jb5rta31GWNglaYCUpKrfrawP9TlK6q4mhA7gZlPakjUGL2zBH5MrpOfcldEFOgKmVkNrbSb /rOyri/4lTY838FYj9yuu+mGqGiaue60Tmm0hK6aYD76vRxjnVaPpIAHOgdcS9qZwChxLid/ jnvWauYEm/FNoWU8AgUvoIx/ytWZcWq85efSZSzXFD6I+QrvBIAzt03ZHzaM7H09c4uM01Mq tsRdApRYwmCg/mz+u/haeNj05FLwMnDZOvzu1lG5BSAVLMNZsmGRK/Ho9hFwD03m8ZCW+7EY NYUYiZuaxKGZABTPlAQC9Q1m+LAanvXKmUE7g7K4/dppTGNnWSd05C0WDbRUvWMSd9YgQCzo WXe8n6iKhobKMae2XyO9XfEaurnxHmjB9JKSuDonhJsqH+0zWpOUjcGaWPli+m2pxa5dthZK WVBr0LCqoB3riRHVOLVURC0rWSFtRlaQNdKGuM77gClwLfb+AufCS4PSTspQMwrsoo6SCIn0 neNnsj1Hnp/vbuNU3Wf+7yI6zSoNkA9JmgEfjIAUQoD7PHpvY4ogxTACN1kFcadjNf4BDXxy DCitzUlivMYistj/6em+VHKhRq8q56PSRQ6ji3MRX6s5A59YI+jZqSr5ELd4PIGK5yWJnGeu FAUls7Y6/oBZaxhjwSISeQJWbquvvCMNWSFhUY1RsdwsTOw53SkYIZcpilkI1tkOdoFfjmvZ 1LPvQRW59lYO37CgbJLXr9dwv8ClcDIfekJnNiOBjaSSvCdrDO6wRw=
  • Ironport-hdrordr: A9a23:QMga46tQxCGHFCWLXo9eAItj7skDeNV00zEX/kB9WHVpm62j+/ xG+c5x6faaslkssR0b9+xoWpPhfZqsz/9ICOAqVN/JMTUO01HYT72Kg7GSpgHIKmnT8fNcyL clU4UWMqyVMbGit7eZ3DWF
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 23/08/2023 8:04 am, Federico Serafini wrote:
> Make function declarations and definitions consistent to address
> violations of MISRA C:2012 Rule 8.3 ("All declarations of an object or
> function shall use the same names and type qualifiers").
>
> No functional change.
>
> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> ---
> Changes in v2:
> - change compat_grant_table_op() definition instead of the declaration;
> - use unsigned int for multicall()'s parameter in accordance with XEN coding
>   style.

Nack.

You were correct in v1, and the request to change it was erroneous.

> diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
> index 6d361ddfce..d1892fd14f 100644
> --- a/xen/include/hypercall-defs.c
> +++ b/xen/include/hypercall-defs.c
> @@ -135,8 +135,8 @@ xenoprof_op(int op, void *arg)
>  #ifdef CONFIG_COMPAT
>  prefix: compat
>  set_timer_op(uint32_t lo, int32_t hi)
> -multicall(multicall_entry_compat_t *call_list, uint32_t nr_calls)
> -memory_op(unsigned int cmd, void *arg)
> +multicall(multicall_entry_compat_t *call_list, unsigned int nr_calls)

This, unfortunately for many reasons, is an ABI with guests.

It is buggy that the entire file doesn't use unsigned long (i.e. one
full GPR width) to begin with.  It these types alone which cause
explicit truncation of the guest-provided hypercall parameter values.

But it is even more buggy to take something that at least truncates to a
fixed width, and replace it with something that explicitly does not have
a fixed width.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.