[Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable

[Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable

[Shraddha Barke]

Compress lines and remove the variable .

Change made using Coccinelle script

@@
expression ret;
@@
- if (ret) return ret;
- return 0;
+ return ret;
@@
local idexpression ret;
expression e;
@@
- ret = e;
- return ret;
+ return e;
@@
type T; identifier i;
@@
- T i;
... when != i

Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
---
 target-microblaze/op_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index d2b3624..d2f411a 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -151,9 +151,7 @@ uint32_t helper_clz(uint32_t t0)
 
 uint32_t helper_carry(uint32_t a, uint32_t b, uint32_t cf)
 {
-    uint32_t ncf;
-    ncf = compute_carry(a, b, cf);
-    return ncf;
+    return compute_carry(a, b, cf);
 }
 
 static inline int div_prepare(CPUMBState *env, uint32_t a, uint32_t b)
-- 
2.1.4

[Qemu-devel] [PATCH 2/3] Hw: timer: Remove unnecessary variable

[Shraddha Barke]

Compress lines and remove the variable ret.

Change made using Coccinelle script

@@
expression ret;
@@
- if (ret) return ret;
- return 0;
+ return ret;
@@
local idexpression ret;
expression e;
@@
- ret = e;
- return ret;
+ return e;
@@
type T; identifier i;
@@
- T i;
... when != i

Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
---
 hw/timer/tusb6010.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/timer/tusb6010.c b/hw/timer/tusb6010.c
index 459c748..ba01050 100644
--- a/hw/timer/tusb6010.c
+++ b/hw/timer/tusb6010.c
@@ -321,7 +321,6 @@ static uint32_t tusb_async_readw(void *opaque, hwaddr addr)
     TUSBState *s = (TUSBState *) opaque;
     int offset = addr & 0xfff;
     int epnum;
-    uint32_t ret;
 
     switch (offset) {
     case TUSB_DEV_CONF:
@@ -338,12 +337,11 @@ static uint32_t tusb_async_readw(void *opaque, hwaddr addr)
         return 0x00;	/* TODO */
 
     case TUSB_DEV_OTG_STAT:
-        ret = s->otg_status;
 #if 0
         if (!(s->prcm_mngmt & TUSB_PRCM_MNGMT_OTG_VBUS_DET_EN))
             ret &= ~TUSB_DEV_OTG_STAT_VBUS_VALID;
 #endif
-        return ret;
+        return s->otg_status;
     case TUSB_DEV_OTG_TIMER:
         return s->otg_timer_val;
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH 2/3] Hw: timer: Remove unnecessary variable

[Peter Maydell]

On 25 September 2015 at 01:37, Shraddha Barke <shraddha.6596@gmail.com> wrote:
> Compress lines and remove the variable ret.
>
> Change made using Coccinelle script

> diff --git a/hw/timer/tusb6010.c b/hw/timer/tusb6010.c
> index 459c748..ba01050 100644
> --- a/hw/timer/tusb6010.c
> +++ b/hw/timer/tusb6010.c
> @@ -321,7 +321,6 @@ static uint32_t tusb_async_readw(void *opaque, hwaddr addr)
>      TUSBState *s = (TUSBState *) opaque;
>      int offset = addr & 0xfff;
>      int epnum;
> -    uint32_t ret;
>
>      switch (offset) {
>      case TUSB_DEV_CONF:
> @@ -338,12 +337,11 @@ static uint32_t tusb_async_readw(void *opaque, hwaddr addr)
>          return 0x00;   /* TODO */
>
>      case TUSB_DEV_OTG_STAT:
> -        ret = s->otg_status;
>  #if 0
>          if (!(s->prcm_mngmt & TUSB_PRCM_MNGMT_OTG_VBUS_DET_EN))
>              ret &= ~TUSB_DEV_OTG_STAT_VBUS_VALID;
>  #endif
> -        return ret;
> +        return s->otg_status;
>      case TUSB_DEV_OTG_TIMER:
>          return s->otg_timer_val;
>

This change would break the #if-0'd out code if we ever reenabled it.

-- PMM

[Qemu-devel] [PATCH 3/3] Target-ppc: Remove unnecessary variable

[Shraddha Barke]

Compress lines and remove the variable.

Change made using Coccinelle script

@@
expression ret;
@@
- if (ret) return ret;
- return 0;
+ return ret;
@@
local idexpression ret;
expression e;
@@
- ret = e;
- return ret;
+ return e;
@@
type T; identifier i;
@@
- T i;
... when != i

Signed-off-by: Shraddha Barke <shraddha.6596@gmail.com>
---
 target-ppc/kvm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 110436d..a3fb4c0 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1782,8 +1782,7 @@ uint32_t kvmppc_get_tbfreq(void)
 
     ns++;
 
-    retval = atoi(ns);
-    return retval;
+    return atoi(ns);
 }
 
 bool kvmppc_get_host_serial(char **value)
-- 
2.1.4

Re: [Qemu-devel] [PATCH 3/3] Target-ppc: Remove unnecessary variable

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

On 09/25/2015 02:37 AM, Shraddha Barke wrote:
> Compress lines and remove the variable.
> 

> +++ b/target-ppc/kvm.c
> @@ -1782,8 +1782,7 @@ uint32_t kvmppc_get_tbfreq(void)
>  
>      ns++;
>  
> -    retval = atoi(ns);
> -    return retval;
> +    return atoi(ns);

atoi() is lousy; it cannot properly detect user input errors.  This
should probably be converted to use the appropriate qemu_strtol variant
instead.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/3] Target-ppc: Remove unnecessary variable

[Paolo Bonzini]

On 26/09/2015 18:15, Eric Blake wrote:
> On 09/25/2015 02:37 AM, Shraddha Barke wrote:
>> Compress lines and remove the variable.
>>
> 
>> +++ b/target-ppc/kvm.c
>> @@ -1782,8 +1782,7 @@ uint32_t kvmppc_get_tbfreq(void)
>>  
>>      ns++;
>>  
>> -    retval = atoi(ns);
>> -    return retval;
>> +    return atoi(ns);
> 
> atoi() is lousy; it cannot properly detect user input errors.  This
> should probably be converted to use the appropriate qemu_strtol variant
> instead.

But it's more or less okay here, it's parsing /proc/cpuinfo.

Paolo

Re: [Qemu-devel] [PATCH 3/3] Target-ppc: Remove unnecessary variable

[Michael Tokarev]

25.09.2015 11:37, Shraddha Barke wrote:
> Compress lines and remove the variable.

Applied to -trivial, removing this piece of commit message:

---
> Change made using Coccinelle script
> 
> @@
> expression ret;
> @@
> - if (ret) return ret;
> - return 0;
> + return ret;
> @@
> local idexpression ret;
> expression e;
> @@
> - ret = e;
> - return ret;
> + return e;
> @@
> type T; identifier i;
> @@
> - T i;
> ... when != i
---

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable

[Michael Tokarev]

25.09.2015 11:37, Shraddha Barke wrote:
> Compress lines and remove the variable .

Applied to -trivial, removing this piece of commit message:

---
> Change made using Coccinelle script
> 
> @@
> expression ret;
> @@
> - if (ret) return ret;
> - return 0;
> + return ret;
> @@
> local idexpression ret;
> expression e;
> @@
> - ret = e;
> - return ret;
> + return e;
> @@
> type T; identifier i;
> @@
> - T i;
> ... when != i
---

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable

[Markus Armbruster]

Michael Tokarev <mjt@tls.msk.ru> writes:

> 25.09.2015 11:37, Shraddha Barke wrote:
>> Compress lines and remove the variable .
>
> Applied to -trivial, removing this piece of commit message:
>
> ---
>> Change made using Coccinelle script
>> 
>> @@
>> expression ret;
>> @@
>> - if (ret) return ret;
>> - return 0;
>> + return ret;
>> @@
>> local idexpression ret;
>> expression e;
>> @@
>> - ret = e;
>> - return ret;
>> + return e;
>> @@
>> type T; identifier i;
>> @@
>> - T i;
>> ... when != i
> ---

Why?  I like having the semantic patch in the commit message when
there's any chance we'll want do the same mechanical change again later.

You could save space and include it by reference, though: "Same
Coccinelle semantic patch as is commit 74c373e".

Re: [Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable

[Michael Tokarev]

05.10.2015 08:18, Markus Armbruster wrote:
> Michael Tokarev <mjt@tls.msk.ru> writes:
> 
>> 25.09.2015 11:37, Shraddha Barke wrote:
>>> Compress lines and remove the variable .
>>
>> Applied to -trivial, removing this piece of commit message:
>>
>> ---
>>> Change made using Coccinelle script
[..snip..]
>> ---
> 
> Why?  I like having the semantic patch in the commit message when
> there's any chance we'll want do the same mechanical change again later.
> 
> You could save space and include it by reference, though: "Same
> Coccinelle semantic patch as is commit 74c373e".

git commit messages aren't good documentation for various scripts
like this, this info will be lost in the noize.  If it might be
better to keep such scripts in a separate file where it is easier
to find, or in a wiki page on the site. The key point is where to
find the info, git log is difficult for that, especially when you
don't know what to search for or that such a script exists in
there in the first place.

On the other hand, when git log is cluttered by such a long messages
for such small changes, it becomes more difficult to find info which
you really look in git log -- namely, which changes were made that
might have introduced this regression, things like that.

So to me, the shorter and cleaner the commit message is, the better.

BTW, I've no idea why this email has been Cc'd to kvm@vger :)

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable

[Peter Maydell]

On 5 October 2015 at 08:18, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 05.10.2015 08:18, Markus Armbruster wrote:
>> Why?  I like having the semantic patch in the commit message when
>> there's any chance we'll want do the same mechanical change again later.
>>
>> You could save space and include it by reference, though: "Same
>> Coccinelle semantic patch as is commit 74c373e".
>
> git commit messages aren't good documentation for various scripts
> like this, this info will be lost in the noize.  If it might be
> better to keep such scripts in a separate file where it is easier
> to find, or in a wiki page on the site. The key point is where to
> find the info, git log is difficult for that, especially when you
> don't know what to search for or that such a script exists in
> there in the first place.
>
> On the other hand, when git log is cluttered by such a long messages
> for such small changes, it becomes more difficult to find info which
> you really look in git log -- namely, which changes were made that
> might have introduced this regression, things like that.

I think it can be useful when you're looking at a commit
to know that it was automatically created, especially if
it's a big commit. It means that if you're looking for
a bug in it you can concentrate on the script that created
it rather than the possibly large set of changes it produced,
or if you're trying to cherry-pick it into another branch you
can just apply the script instead.

In a commit with a change this small it's not very significant
either way, though.

thanks
-- PMM