| Message ID | 1346703803-22003-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Under Review |
| Headers | show |
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
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
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
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
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
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 > >
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.
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.
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) {
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(-)