hvf: drop unused variable

[Qemu-devel] [PATCH] hvf: drop unused variable

Posted by Paolo Bonzini 7 years, 1 month ago

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/hvf/hvf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 5db167df98..9f52bc413a 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -72,7 +72,6 @@
 #include "sysemu/sysemu.h"
 #include "target/i386/cpu.h"
 
-pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
 HVFState *hvf_state;
 int hvf_disabled = 1;
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH] hvf: drop unused variable

Posted by Thomas Huth 7 years, 1 month ago

On 2018-09-18 11:28, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target/i386/hvf/hvf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 5db167df98..9f52bc413a 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -72,7 +72,6 @@
>  #include "sysemu/sysemu.h"
>  #include "target/i386/cpu.h"
>  
> -pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
>  HVFState *hvf_state;
>  int hvf_disabled = 1;
>  
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [Qemu-trivial] [PATCH] hvf: drop unused variable

Posted by Philippe Mathieu-Daudé 7 years, 1 month ago

On 9/18/18 11:28 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  target/i386/hvf/hvf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 5db167df98..9f52bc413a 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -72,7 +72,6 @@
>  #include "sysemu/sysemu.h"
>  #include "target/i386/cpu.h"
>  
> -pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
>  HVFState *hvf_state;
>  int hvf_disabled = 1;

I'm surprised we never got a warning for this...
We do use:

  -Wunused-but-set-variable
    Warn whenever a local variable is assigned to, but otherwise
    unused (aside from its declaration).  This warning is enabled
    by -Wall.

Last Travis CI build: https://travis-ci.org/qemu/qemu/jobs/429242442

ProductName:	Mac OS X
ProductVersion:	10.13.3
BuildVersion:	17D102

Target: x86_64-apple-darwin17.4.0
Thread model: posix

$ ./configure
...
KVM support       no
HAX support       yes
HVF support       yes
WHPX support      no
TCG support       yes

C compiler        clang
LDFLAGS           -framework Hypervisor -m64 -framework CoreFoundation
-framework IOKit -arch x86_64 -g
CFLAGS            -O2 -g
QEMU_CFLAGS       -I/usr/local/Cellar/pixman/0.34.0_1/include/pixman-1
-I$(SRC_PATH)/dtc/libfdt -D_REENTRANT
-I/usr/local/Cellar/glib/2.58.0_1/include/glib-2.0
-I/usr/local/Cellar/glib/2.58.0_1/lib/glib-2.0/include
-I/usr/local/opt/gettext/include -I/usr/local/Cellar/pcre/8.42/include
-m64 -mcx16 -DOS_OBJECT_USE_OBJC=0 -arch x86_64 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv
-Wno-error=address-of-packed-member -Wno-string-plus-int
-Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels
-Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body
-Wnested-externs -Wformat-security -Wformat-y2k -Winit-self
-Wignored-qualifiers -Wold-style-definition -Wtype-limits
-fstack-protector-strong -Wno-missing-braces
-I/usr/local/Cellar/gnutls/3.5.18/include
-I/usr/local/Cellar/nettle/3.4/include
-I/usr/local/Cellar/libtasn1/4.13/include
-I/usr/local/Cellar/p11-kit/0.23.12/include/p11-kit-1
-I/usr/local/Cellar/nettle/3.4/include
-I/usr/local/Cellar/libpng/1.6.34/include/libpng16
-I$(SRC_PATH)/capstone/include

...
  CC      x86_64-softmmu/target/i386/hax-darwin.o
  CC      x86_64-softmmu/target/i386/hvf/hvf.o
  CC      x86_64-softmmu/target/i386/hvf/x86.o
  CC      x86_64-softmmu/target/i386/hvf/x86_cpuid.o
  CC      x86_64-softmmu/target/i386/hvf/x86_decode.o
  CC      x86_64-softmmu/target/i386/hvf/x86_descr.o
  CC      x86_64-softmmu/target/i386/hvf/x86_emu.o
  CC      x86_64-softmmu/target/i386/hvf/x86_flags.o
  CC      x86_64-softmmu/target/i386/hvf/x86_mmu.o
  CC      x86_64-softmmu/target/i386/hvf/x86hvf.o
  CC      x86_64-softmmu/target/i386/hvf/x86_task.o
  LINK    x86_64-softmmu/qemu-system-x86_64
  REZ     x86_64-softmmu/qemu-system-x86_64
  SETFILE x86_64-softmmu/qemu-system-x86_64

So this code is covered.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Regards,

Phil.

Re: [Qemu-devel] [Qemu-trivial] [PATCH] hvf: drop unused variable

Posted by Thomas Huth 7 years, 1 month ago

On 2018-09-19 12:30, Philippe Mathieu-Daudé wrote:
> On 9/18/18 11:28 AM, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  target/i386/hvf/hvf.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
>> index 5db167df98..9f52bc413a 100644
>> --- a/target/i386/hvf/hvf.c
>> +++ b/target/i386/hvf/hvf.c
>> @@ -72,7 +72,6 @@
>>  #include "sysemu/sysemu.h"
>>  #include "target/i386/cpu.h"
>>  
>> -pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
>>  HVFState *hvf_state;
>>  int hvf_disabled = 1;
> 
> I'm surprised we never got a warning for this...

The variable is not marked with "static", so this is a global variable.
When compiling this file, the compiler can not know whether another file
uses "extern pthread_rwlock_t mem_lock" to access this variable, so it
can not know whether it is used or not.

To detect such unused global variables, you need to do some magic with
the linker instead. See the following URL for more information:

https://flameeyes.blog/2008/01/17/today-how-to-identify-unused-exported-functions-and-variables/

 Thomas