[v2,01/20] hw/i386/pc: Use unsigned type to index arrays
diff mbox series

Message ID 20190613143446.23937-2-philmd@redhat.com
State New
Headers show
Series
  • hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine
Related show

Commit Message

Philippe Mathieu-Daudé June 13, 2019, 2:34 p.m. UTC
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc.c         | 5 +++--
 include/hw/i386/pc.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Michael S. Tsirkin June 20, 2019, 3:27 p.m. UTC | #1
On Thu, Jun 13, 2019 at 04:34:27PM +0200, Philippe Mathieu-Daudé wrote:
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Motivation?  Is this a bugfix?


> ---
>  hw/i386/pc.c         | 5 +++--
>  include/hw/i386/pc.h | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2c5446b095..bb3c74f4ca 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -874,7 +874,7 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
>  
>  int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
>  {
> -    int index = le32_to_cpu(e820_reserve.count);
> +    unsigned int index = le32_to_cpu(e820_reserve.count);
>      struct e820_entry *entry;
>  
>      if (type != E820_RAM) {
> @@ -906,7 +906,8 @@ int e820_get_num_entries(void)
>      return e820_entries;
>  }
>  
> -bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length)
> +bool e820_get_entry(unsigned int idx, uint32_t type,
> +                    uint64_t *address, uint64_t *length)
>  {
>      if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
>          *address = le64_to_cpu(e820_table[idx].address);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index a7d0b87166..3b3a0d6e59 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -291,7 +291,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
>  
>  int e820_add_entry(uint64_t, uint64_t, uint32_t);
>  int e820_get_num_entries(void);
> -bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> +bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
>  
>  extern GlobalProperty pc_compat_4_0_1[];
>  extern const size_t pc_compat_4_0_1_len;
> -- 
> 2.20.1
Philippe Mathieu-Daudé June 21, 2019, 2:44 p.m. UTC | #2
On 6/20/19 5:27 PM, Michael S. Tsirkin wrote:
> On Thu, Jun 13, 2019 at 04:34:27PM +0200, Philippe Mathieu-Daudé wrote:
>> Reviewed-by: Li Qiang <liq3ea@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Motivation?  Is this a bugfix?

Apparently I started to work on this series after "chardev: Convert
qemu_chr_write() to take a size_t argument" [*] for which I had these
extra warnings:

  --extra-cflags=-Wtype-limits\
                 -Wsign-compare\
                 -Wno-error=sign-compare

[*] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05229.html

>> ---
>>  hw/i386/pc.c         | 5 +++--
>>  include/hw/i386/pc.h | 2 +-
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 2c5446b095..bb3c74f4ca 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -874,7 +874,7 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
>>  
>>  int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
>>  {
>> -    int index = le32_to_cpu(e820_reserve.count);
>> +    unsigned int index = le32_to_cpu(e820_reserve.count);
>>      struct e820_entry *entry;
>>  
>>      if (type != E820_RAM) {
>> @@ -906,7 +906,8 @@ int e820_get_num_entries(void)
>>      return e820_entries;
>>  }
>>  
>> -bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length)
>> +bool e820_get_entry(unsigned int idx, uint32_t type,
>> +                    uint64_t *address, uint64_t *length)
>>  {
>>      if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
>>          *address = le64_to_cpu(e820_table[idx].address);

And here I wanted to fix:

hw/i386/pc.c:911:13: warning: comparison of integers of different signs:
'int' and 'unsigned int' [-Wsign-compare]
    if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
        ~~~ ^ ~~~~~~~~~~~~
hw/i386/pc.c:972:36: warning: comparison of integers of different signs:
'unsigned int' and 'int' [-Wsign-compare]
    for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
                                 ~ ^ ~~~~~~~~~~~~~~~~~~~~~~
Is it worthwhile?

>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index a7d0b87166..3b3a0d6e59 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -291,7 +291,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
>>  
>>  int e820_add_entry(uint64_t, uint64_t, uint32_t);
>>  int e820_get_num_entries(void);
>> -bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>> +bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
>>  
>>  extern GlobalProperty pc_compat_4_0_1[];
>>  extern const size_t pc_compat_4_0_1_len;
>> -- 
>> 2.20.1

Patch
diff mbox series

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2c5446b095..bb3c74f4ca 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -874,7 +874,7 @@  static void handle_a20_line_change(void *opaque, int irq, int level)
 
 int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
 {
-    int index = le32_to_cpu(e820_reserve.count);
+    unsigned int index = le32_to_cpu(e820_reserve.count);
     struct e820_entry *entry;
 
     if (type != E820_RAM) {
@@ -906,7 +906,8 @@  int e820_get_num_entries(void)
     return e820_entries;
 }
 
-bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length)
+bool e820_get_entry(unsigned int idx, uint32_t type,
+                    uint64_t *address, uint64_t *length)
 {
     if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
         *address = le64_to_cpu(e820_table[idx].address);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index a7d0b87166..3b3a0d6e59 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -291,7 +291,7 @@  void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
-bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
+bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
 
 extern GlobalProperty pc_compat_4_0_1[];
 extern const size_t pc_compat_4_0_1_len;