Patchwork [3/3] Avoid Wunsed-but-set warnings (or errors in case of Werror)

login
register
mail settings
Submitter Raghavendra D Prabhu
Date July 4, 2011, 10 p.m.
Message ID <2cb2082c25eadc24d1e22ebbd4293e3e6b53c832.1309816302.git.rprabhu@wnohang.net>
Download mbox | patch
Permalink /patch/103175/
State New
Headers show

Comments

Raghavendra D Prabhu - July 4, 2011, 10 p.m.
In a few cases, variable attributed 'unused' has been added, in other cases
unused variable has been either removed or commented out.

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 hw/device-assignment.c |    6 +++---
 simpletrace.c          |    2 +-
 xen-mapcache.c         |    7 ++-----
 3 files changed, 6 insertions(+), 9 deletions(-)
Markus Armbruster - July 5, 2011, 6:15 a.m.
Typo in subject: "unsed".  The warning is spelled
"unused-but-set-variable", the option "-Wunused-but-set-variable".

Raghavendra D Prabhu <raghu.prabhu13@gmail.com> writes:

> In a few cases, variable attributed 'unused' has been added, in other cases
> unused variable has been either removed or commented out.
>
> Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
> ---
>  hw/device-assignment.c |    6 +++---
>  simpletrace.c          |    2 +-
>  xen-mapcache.c         |    7 ++-----
>  3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 36ad6b0..19a59b4 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1654,7 +1654,7 @@ static void reset_assigned_device(DeviceState *dev)
>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>      char reset_file[64];
>      const char reset[] = "1";
> -    int fd, ret;
> +    int fd, __attribute__((unused)) ret;
>  
>      snprintf(reset_file, sizeof(reset_file),
>               "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/reset",

What about (void)write() and do away with ret?

> @@ -1682,7 +1682,7 @@ static void reset_assigned_device(DeviceState *dev)
>  static int assigned_initfn(struct PCIDevice *pci_dev)
>  {
>      AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> -    uint8_t e_device, e_intx;
> +    uint8_t e_intx;
>      int r;
>  
>      if (!kvm_enabled()) {
> @@ -1709,7 +1709,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
>          goto out;
>  
>      /* handle interrupt routing */
> -    e_device = (dev->dev.devfn >> 3) & 0x1f;
> +    /*e_device = (dev->dev.devfn >> 3) & 0x1f;*/
>      e_intx = dev->dev.config[0x3d] - 1;
>      dev->intpin = e_intx;
>      dev->run = 0;
> diff --git a/simpletrace.c b/simpletrace.c
> index f1dbb5e..2ce9cff 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -119,7 +119,7 @@ static void *writeout_thread(void *opaque)
>      TraceRecord record;
>      unsigned int writeout_idx = 0;
>      unsigned int num_available, idx;
> -    size_t unused;
> +    size_t __attribute__((unused)) unused;
>  
>      for (;;) {
>          wait_for_trace_records_available();

Same here.

[...]
Peter Maydell - July 5, 2011, 7:02 a.m.
On 5 July 2011 07:15, Markus Armbruster <armbru@redhat.com> wrote:
>> +    int fd, __attribute__((unused)) ret;
>>
>>      snprintf(reset_file, sizeof(reset_file),
>>               "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/reset",
>
> What about (void)write() and do away with ret?

If 'ret' has been used to silence compiler warnings about functions
which have been declared with attribute __warn_unused_result__
(eg write() and various other libc functions) then "(void)write()"
is insufficient -- gcc requires the variable.

-- PMM
Markus Armbruster - July 5, 2011, 7:49 a.m.
Peter Maydell <peter.maydell@linaro.org> writes:

> On 5 July 2011 07:15, Markus Armbruster <armbru@redhat.com> wrote:
>>> +    int fd, __attribute__((unused)) ret;
>>>
>>>      snprintf(reset_file, sizeof(reset_file),
>>>               "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/reset",
>>
>> What about (void)write() and do away with ret?
>
> If 'ret' has been used to silence compiler warnings about functions
> which have been declared with attribute __warn_unused_result__
> (eg write() and various other libc functions) then "(void)write()"
> is insufficient -- gcc requires the variable.

gcc being silly.  Oh well.
Paolo Bonzini - July 5, 2011, 8:05 a.m.
On 07/05/2011 09:49 AM, Markus Armbruster wrote:
> >  If 'ret' has been used to silence compiler warnings about functions
> >  which have been declared with attribute __warn_unused_result__
> >  (eg write() and various other libc functions) then "(void)write()"
> >  is insufficient -- gcc requires the variable.
>
> gcc being silly.  Oh well.

In this particular case I think that the return value should be checked. 
  It's good if something is printed in the log saying that the reset 
wasn't done for some reason---even if it is just defensive.

The really silly thing is in glibc, not gcc.  __warn_unused_result__ was 
added to fwrite, where you have no certainty that the write has been 
done, but not to either fclose or fflush.  Oh well...

Paolo

Patch

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 36ad6b0..19a59b4 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1654,7 +1654,7 @@  static void reset_assigned_device(DeviceState *dev)
     AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     char reset_file[64];
     const char reset[] = "1";
-    int fd, ret;
+    int fd, __attribute__((unused)) ret;
 
     snprintf(reset_file, sizeof(reset_file),
              "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/reset",
@@ -1682,7 +1682,7 @@  static void reset_assigned_device(DeviceState *dev)
 static int assigned_initfn(struct PCIDevice *pci_dev)
 {
     AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
-    uint8_t e_device, e_intx;
+    uint8_t e_intx;
     int r;
 
     if (!kvm_enabled()) {
@@ -1709,7 +1709,7 @@  static int assigned_initfn(struct PCIDevice *pci_dev)
         goto out;
 
     /* handle interrupt routing */
-    e_device = (dev->dev.devfn >> 3) & 0x1f;
+    /*e_device = (dev->dev.devfn >> 3) & 0x1f;*/
     e_intx = dev->dev.config[0x3d] - 1;
     dev->intpin = e_intx;
     dev->run = 0;
diff --git a/simpletrace.c b/simpletrace.c
index f1dbb5e..2ce9cff 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -119,7 +119,7 @@  static void *writeout_thread(void *opaque)
     TraceRecord record;
     unsigned int writeout_idx = 0;
     unsigned int num_available, idx;
-    size_t unused;
+    size_t __attribute__((unused)) unused;
 
     for (;;) {
         wait_for_trace_records_available();
diff --git a/xen-mapcache.c b/xen-mapcache.c
index fac47cd..94642bc 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -166,7 +166,7 @@  static void qemu_remap_bucket(MapCacheEntry *entry,
 
 uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, uint8_t lock)
 {
-    MapCacheEntry *entry, *pentry = NULL;
+    MapCacheEntry *entry;
     target_phys_addr_t address_index  = phys_addr >> MCACHE_BUCKET_SHIFT;
     target_phys_addr_t address_offset = phys_addr & (MCACHE_BUCKET_SIZE - 1);
     target_phys_addr_t __size = size;
@@ -192,12 +192,10 @@  uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, u
             (entry->paddr_index != address_index || entry->size != __size ||
              !test_bits(address_offset >> XC_PAGE_SHIFT, size >> XC_PAGE_SHIFT,
                  entry->valid_mapping))) {
-        pentry = entry;
         entry = entry->next;
     }
     if (!entry) {
         entry = qemu_mallocz(sizeof (MapCacheEntry));
-        pentry->next = entry;
         qemu_remap_bucket(entry, __size, address_index);
     } else if (!entry->lock) {
         if (!entry->vaddr_base || entry->paddr_index != address_index ||
@@ -232,7 +230,7 @@  uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, u
 
 ram_addr_t qemu_ram_addr_from_mapcache(void *ptr)
 {
-    MapCacheEntry *entry = NULL, *pentry = NULL;
+    MapCacheEntry *entry = NULL;
     MapCacheRev *reventry;
     target_phys_addr_t paddr_index;
     target_phys_addr_t size;
@@ -258,7 +256,6 @@  ram_addr_t qemu_ram_addr_from_mapcache(void *ptr)
 
     entry = &mapcache->entry[paddr_index % mapcache->nr_buckets];
     while (entry && (entry->paddr_index != paddr_index || entry->size != size)) {
-        pentry = entry;
         entry = entry->next;
     }
     if (!entry) {