[Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Weil]

Report from smatch:
sparc-dis.c:2664 build_hash_table(14) info:
 redundant null check on hash_buf calling free()

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

Coding style was not fixed.

- sw

 sparc-dis.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sparc-dis.c b/sparc-dis.c
index cdd337a..ef28835 100644
--- a/sparc-dis.c
+++ b/sparc-dis.c
@@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
 
   memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
   memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
-  if (hash_buf != NULL)
-    free (hash_buf);
+  free (hash_buf);
   hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
   for (i = num_opcodes - 1; i >= 0; --i)
     {
-- 
1.7.10

Re: [Qemu-devel] [Qemu-trivial] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Hajnoczi]

On Mon, Sep 03, 2012 at 10:23:23PM +0200, Stefan Weil wrote:
> Report from smatch:
> sparc-dis.c:2664 build_hash_table(14) info:
>  redundant null check on hash_buf calling free()
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> Coding style was not fixed.
> 
> - sw
> 
>  sparc-dis.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Andreas Färber]

Am 03.09.2012 22:23, schrieb Stefan Weil:
> Report from smatch:
> sparc-dis.c:2664 build_hash_table(14) info:
>  redundant null check on hash_buf calling free()
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> Coding style was not fixed.
> 
> - sw
> 
>  sparc-dis.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/sparc-dis.c b/sparc-dis.c
> index cdd337a..ef28835 100644
> --- a/sparc-dis.c
> +++ b/sparc-dis.c
> @@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
>  
>    memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
>    memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
> -  if (hash_buf != NULL)
> -    free (hash_buf);
> +  free (hash_buf);
>    hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
>    for (i = num_opcodes - 1; i >= 0; --i)
>      {

*-dis sounds like binutils - did upstream drop the if, too?
If not, then diverging for a non-issue does not seem necessary.

Having said that, it's amazing what fixes you come up with. Did you
describe the use of that tool in some posting or in the Wiki?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Hajnoczi]

On Wed, Sep 5, 2012 at 4:51 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 03.09.2012 22:23, schrieb Stefan Weil:
>> Report from smatch:
>> sparc-dis.c:2664 build_hash_table(14) info:
>>  redundant null check on hash_buf calling free()
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> Coding style was not fixed.
>>
>> - sw
>>
>>  sparc-dis.c |    3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/sparc-dis.c b/sparc-dis.c
>> index cdd337a..ef28835 100644
>> --- a/sparc-dis.c
>> +++ b/sparc-dis.c
>> @@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
>>
>>    memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
>>    memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
>> -  if (hash_buf != NULL)
>> -    free (hash_buf);
>> +  free (hash_buf);
>>    hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
>>    for (i = num_opcodes - 1; i >= 0; --i)
>>      {
>
> *-dis sounds like binutils - did upstream drop the if, too?
> If not, then diverging for a non-issue does not seem necessary.

Ah, good point.  I don't think we should apply this patch :P.

Dropping the patch from qemu-trivial until this discussion finishes.

Stefan

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Weil]

Am 05.09.2012 18:55, schrieb Stefan Hajnoczi:
> On Wed, Sep 5, 2012 at 4:51 PM, Andreas Färber<afaerber@suse.de>  wrote:
>> Am 03.09.2012 22:23, schrieb Stefan Weil:
>>> Report from smatch:
>>> sparc-dis.c:2664 build_hash_table(14) info:
>>>   redundant null check on hash_buf calling free()
>>>
>>> Signed-off-by: Stefan Weil<sw@weilnetz.de>
>>> ---
>>>
>>> Coding style was not fixed.
>>>
>>> - sw
>>>
>>>   sparc-dis.c |    3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/sparc-dis.c b/sparc-dis.c
>>> index cdd337a..ef28835 100644
>>> --- a/sparc-dis.c
>>> +++ b/sparc-dis.c
>>> @@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
>>>
>>>     memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
>>>     memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
>>> -  if (hash_buf != NULL)
>>> -    free (hash_buf);
>>> +  free (hash_buf);
>>>     hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
>>>     for (i = num_opcodes - 1; i>= 0; --i)
>>>       {
>>
>> *-dis sounds like binutils - did upstream drop the if, too?
>> If not, then diverging for a non-issue does not seem necessary.
>
> Ah, good point.  I don't think we should apply this patch :P.
>
> Dropping the patch from qemu-trivial until this discussion finishes.
>
> Stefan

AFAIK, binutils moved to GPL 3. Therefore I don't expect that
QEMU will update to upstream in the next years.

We'll have to maintain the code which we have.

Try "git log *-dis.c" or "gitk *-dis.c": there are already lots
of more trivial changes which got applied to the disassembler files.

=> The patch should be applied.

Regards,
Stefan

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Weil]

Am 05.09.2012 19:15, schrieb Stefan Weil:
>
> AFAIK, binutils moved to GPL 3. Therefore I don't expect that
> QEMU will update to upstream in the next years.
>
> We'll have to maintain the code which we have.
>
> Try "git log *-dis.c" or "gitk *-dis.c": there are already lots
> of more trivial changes which got applied to the disassembler files.
>
> => The patch should be applied.
>
> Regards,
> Stefan

Here is some additional information:

binutils switched from GPL 2 to GPL 3 with version 2.18:

Changes in 2.18:

* The binutils sources are now released under version 3 of the GNU General
   Public License.


sparc-dis.c is already based on 2.17, so we won't get anything newer.
Even the latest version still uses the redundant NULL check, so I
can send my patch to the binutils maintainers, too.

Regards,
Stefan

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Weil]

Am 05.09.2012 19:45, schrieb Stefan Weil:
> Am 05.09.2012 19:15, schrieb Stefan Weil:
>>
>> AFAIK, binutils moved to GPL 3. Therefore I don't expect that
>> QEMU will update to upstream in the next years.
>>
>> We'll have to maintain the code which we have.
>>
>> Try "git log *-dis.c" or "gitk *-dis.c": there are already lots
>> of more trivial changes which got applied to the disassembler files.
>>
>> => The patch should be applied.
>>
>> Regards,
>> Stefan
>
> Here is some additional information:
>
> binutils switched from GPL 2 to GPL 3 with version 2.18:
>
> Changes in 2.18:
>
> * The binutils sources are now released under version 3 of the GNU 
> General
>   Public License.
>
>
> sparc-dis.c is already based on 2.17, so we won't get anything newer.
> Even the latest version still uses the redundant NULL check, so I
> can send my patch to the binutils maintainers, too.
>
> Regards,
> Stefan
>


Ping? If nobody objects, I suggest to apply the patch via qemu-trivial.

Regards

Stefan W.

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Andreas Färber]

Am 20.09.2012 19:03, schrieb Stefan Weil:
> Am 05.09.2012 19:45, schrieb Stefan Weil:
>> Am 05.09.2012 19:15, schrieb Stefan Weil:
>>>
>>> AFAIK, binutils moved to GPL 3. Therefore I don't expect that
>>> QEMU will update to upstream in the next years.
>>>
>>> We'll have to maintain the code which we have.
>>>
>>> Try "git log *-dis.c" or "gitk *-dis.c": there are already lots
>>> of more trivial changes which got applied to the disassembler files.
>>>
>>> => The patch should be applied.
>>>
>>> Regards,
>>> Stefan
>>
>> Here is some additional information:
>>
>> binutils switched from GPL 2 to GPL 3 with version 2.18:
>>
>> Changes in 2.18:
>>
>> * The binutils sources are now released under version 3 of the GNU
>> General
>>   Public License.
>>
>>
>> sparc-dis.c is already based on 2.17, so we won't get anything newer.
>> Even the latest version still uses the redundant NULL check, so I
>> can send my patch to the binutils maintainers, too.

So did you? And did they accept it?

Regards,
Andreas

> Ping? If nobody objects, I suggest to apply the patch via qemu-trivial.
> 
> Regards
> 
> Stefan W.

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check

[Stefan Weil]

Am 05.09.2012 17:51, schrieb Andreas Färber:
> Am 03.09.2012 22:23, schrieb Stefan Weil:
>    
>> Report from smatch:
>> sparc-dis.c:2664 build_hash_table(14) info:
>>   redundant null check on hash_buf calling free()
>>
>> Signed-off-by: Stefan Weil<sw@weilnetz.de>
>> ---
>>
>> Coding style was not fixed.
>>
>> - sw
>>
>>   sparc-dis.c |    3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/sparc-dis.c b/sparc-dis.c
>> index cdd337a..ef28835 100644
>> --- a/sparc-dis.c
>> +++ b/sparc-dis.c
>> @@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
>>
>>     memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
>>     memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
>> -  if (hash_buf != NULL)
>> -    free (hash_buf);
>> +  free (hash_buf);
>>     hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
>>     for (i = num_opcodes - 1; i>= 0; --i)
>>       {
>>      
> *-dis sounds like binutils - did upstream drop the if, too?
> If not, then diverging for a non-issue does not seem necessary.
>
> Having said that, it's amazing what fixes you come up with. Did you
> describe the use of that tool in some posting or in the Wiki?
>    

I just added some initial text to http://wiki.qemu.org/Testing.

> Regards,
> Andreas
>
>