| Message ID | 1483981521-6789-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 2017-01-09 18:05, Peter Maydell wrote: > The patch_hypercalls() function sets up a 'patches' > variable and checks it at the end of the function, but > never modifies it in the middle. Remove this dead code, > which seems to have been present since the function was > added in commit e5ad936b0fd7 in 2012. > > (Spotted by Coverity: CID 1005581.) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Perhaps this was unintentionally retained debug code? > It's not clear to me why the ROM should be required to > have exactly zero or two hypercalls in it. In any case > we've been fine without checking the patch count for > five years :-) > > CC'd Jan as original code author just in case he remembers. > --- > hw/i386/kvmvapic.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c > index b30d1b9..418dba1 100644 > --- a/hw/i386/kvmvapic.c > +++ b/hw/i386/kvmvapic.c > @@ -534,7 +534,6 @@ static int patch_hypercalls(VAPICROMState *s) > uint8_t alternates[2]; > const uint8_t *pattern; > const uint8_t *patch; > - int patches = 0; > off_t pos; > uint8_t *rom; > > @@ -565,11 +564,6 @@ static int patch_hypercalls(VAPICROMState *s) > } > > g_free(rom); > - > - if (patches != 0 && patches != 2) { > - return -1; > - } > - > return 0; > } > > Hmm, it looked that dead from day #1 on, and I cannot blame it on the qemu-kvm version as well. Maybe the idea was to count the number of patches we applied and have this as cross-check. But then I forgot to count. It's probably safer to bury this code than to try starting to count to 2 now. Acked-by: Jan Kiszka <jan.kiszka@siemens.com> -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux
09.01.2017 20:05, Peter Maydell wrote: > The patch_hypercalls() function sets up a 'patches' > variable and checks it at the end of the function, but > never modifies it in the middle. Remove this dead code, > which seems to have been present since the function was > added in commit e5ad936b0fd7 in 2012. Applied to -trivial, thank you! /mjt
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index b30d1b9..418dba1 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -534,7 +534,6 @@ static int patch_hypercalls(VAPICROMState *s) uint8_t alternates[2]; const uint8_t *pattern; const uint8_t *patch; - int patches = 0; off_t pos; uint8_t *rom; @@ -565,11 +564,6 @@ static int patch_hypercalls(VAPICROMState *s) } g_free(rom); - - if (patches != 0 && patches != 2) { - return -1; - } - return 0; }
The patch_hypercalls() function sets up a 'patches' variable and checks it at the end of the function, but never modifies it in the middle. Remove this dead code, which seems to have been present since the function was added in commit e5ad936b0fd7 in 2012. (Spotted by Coverity: CID 1005581.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Perhaps this was unintentionally retained debug code? It's not clear to me why the ROM should be required to have exactly zero or two hypercalls in it. In any case we've been fine without checking the patch count for five years :-) CC'd Jan as original code author just in case he remembers. --- hw/i386/kvmvapic.c | 6 ------ 1 file changed, 6 deletions(-) -- 2.7.4