| Message ID | 20090705114057.GE4798@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote: > MSIX present bit is tested incorrectly, and only happens to work because > the bit we are testing is 0x1. Add braces to fix this. > > Reported-by: Blue Swirl <blauwirbel@gmail.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/msix.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/msix.c b/hw/msix.c > index 33549f5..db72cc3 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -298,7 +298,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f) > { > unsigned n = dev->msix_entries_nr; > > - if (!dev->cap_present & QEMU_PCI_CAP_MSIX) > + if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) With the braces comment I meant that while working on the code, you should update it to match CODING_STYLE: if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) { return; } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jul 05, 2009 at 02:48:12PM +0300, Blue Swirl wrote: > On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote: > > MSIX present bit is tested incorrectly, and only happens to work because > > the bit we are testing is 0x1. Add braces to fix this. > > > > Reported-by: Blue Swirl <blauwirbel@gmail.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > hw/msix.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/hw/msix.c b/hw/msix.c > > index 33549f5..db72cc3 100644 > > --- a/hw/msix.c > > +++ b/hw/msix.c > > @@ -298,7 +298,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f) > > { > > unsigned n = dev->msix_entries_nr; > > > > - if (!dev->cap_present & QEMU_PCI_CAP_MSIX) > > + if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) > > With the braces comment I meant that while working on the code, you > should update it to match CODING_STYLE: > if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) { > return; > } Yea ... it's probably better to do this all over the file, not piecewise, though. No?
On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Jul 05, 2009 at 02:48:12PM +0300, Blue Swirl wrote: > > On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote: > > > MSIX present bit is tested incorrectly, and only happens to work because > > > the bit we are testing is 0x1. Add braces to fix this. > > > > > > Reported-by: Blue Swirl <blauwirbel@gmail.com> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > --- > > > hw/msix.c | 2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/hw/msix.c b/hw/msix.c > > > index 33549f5..db72cc3 100644 > > > --- a/hw/msix.c > > > +++ b/hw/msix.c > > > @@ -298,7 +298,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f) > > > { > > > unsigned n = dev->msix_entries_nr; > > > > > > - if (!dev->cap_present & QEMU_PCI_CAP_MSIX) > > > + if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) > > > > With the braces comment I meant that while working on the code, you > > should update it to match CODING_STYLE: > > if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) { > > return; > > } > > > Yea ... it's probably better to do this all over the file, not piecewise, > though. No? I think it's better to do it together with other changes: http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg00925.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/05/2009 02:56 PM, Michael S. Tsirkin wrote: >> With the braces comment I meant that while working on the code, you >> should update it to match CODING_STYLE: >> if (!(dev->cap_present& QEMU_PCI_CAP_MSIX)) { >> return; >> } >> > > Yea ... it's probably better to do this all over the file, not piecewise, > though. No? > No, that just causes churn (and merge conflicts for me). Better to only fix if you have a patch that modifies the same place.
On Sun, Jul 05, 2009 at 03:07:06PM +0300, Avi Kivity wrote: > On 07/05/2009 02:56 PM, Michael S. Tsirkin wrote: >>> With the braces comment I meant that while working on the code, you >>> should update it to match CODING_STYLE: >>> if (!(dev->cap_present& QEMU_PCI_CAP_MSIX)) { >>> return; >>> } >>> >> >> Yea ... it's probably better to do this all over the file, not piecewise, >> though. No? >> > > No, that just causes churn (and merge conflicts for me). Better to only > fix if you have a patch that modifies the same place. OK, I redid these patches with {}. Even though the kernel style is better! ;)
diff --git a/hw/msix.c b/hw/msix.c index 33549f5..db72cc3 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -298,7 +298,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f) { unsigned n = dev->msix_entries_nr; - if (!dev->cap_present & QEMU_PCI_CAP_MSIX) + if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) return; msix_free_irq_entries(dev);
MSIX present bit is tested incorrectly, and only happens to work because the bit we are testing is 0x1. Add braces to fix this. Reported-by: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/msix.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)