[PATCH v4 02/16] pci/shpc: change shpc_get_status() return type to uint8_t

Vladimir Sementsov-Ogievskiy posted 16 patches 2 years, 12 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v4 02/16] pci/shpc: change shpc_get_status() return type to uint8_t
Posted by Vladimir Sementsov-Ogievskiy 2 years, 12 months ago
The result of the function is always one byte. The result is always
assigned to uint8_t variable. Also, shpc_get_status() should be
symmetric to shpc_set_status() which has uint8_t value argument.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/pci/shpc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 1b3f619dc9..5d71569b13 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -123,10 +123,13 @@
 #define SHPC_PCI_TO_IDX(pci_slot) ((pci_slot) - 1)
 #define SHPC_IDX_TO_PHYSICAL(slot) ((slot) + 1)
 
-static uint16_t shpc_get_status(SHPCDevice *shpc, int slot, uint16_t msk)
+static uint8_t shpc_get_status(SHPCDevice *shpc, int slot, uint16_t msk)
 {
     uint8_t *status = shpc->config + SHPC_SLOT_STATUS(slot);
-    return (pci_get_word(status) & msk) >> ctz32(msk);
+    uint16_t result = (pci_get_word(status) & msk) >> ctz32(msk);
+
+    assert(result <= UINT8_MAX);
+    return result;
 }
 
 static void shpc_set_status(SHPCDevice *shpc,
-- 
2.34.1
Re: [PATCH v4 02/16] pci/shpc: change shpc_get_status() return type to uint8_t
Posted by Anton Kuchin 2 years, 12 months ago
On 13/02/2023 16:00, Vladimir Sementsov-Ogievskiy wrote:
> The result of the function is always one byte. The result is always
> assigned to uint8_t variable. Also, shpc_get_status() should be
> symmetric to shpc_set_status() which has uint8_t value argument.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   hw/pci/shpc.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 1b3f619dc9..5d71569b13 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -123,10 +123,13 @@
>   #define SHPC_PCI_TO_IDX(pci_slot) ((pci_slot) - 1)
>   #define SHPC_IDX_TO_PHYSICAL(slot) ((slot) + 1)
>   
> -static uint16_t shpc_get_status(SHPCDevice *shpc, int slot, uint16_t msk)
> +static uint8_t shpc_get_status(SHPCDevice *shpc, int slot, uint16_t msk)
>   {
>       uint8_t *status = shpc->config + SHPC_SLOT_STATUS(slot);
> -    return (pci_get_word(status) & msk) >> ctz32(msk);
> +    uint16_t result = (pci_get_word(status) & msk) >> ctz32(msk);
> +
> +    assert(result <= UINT8_MAX);
> +    return result;
>   }
>   
>   static void shpc_set_status(SHPCDevice *shpc,
Reviewed-by: Anton Kuchin <antonkuchin@yandex-team.ru>