| Message ID | 20091001055144.GC5068@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
"Michael S. Tsirkin" <mst@redhat.com> writes: > MULTI_REQ is never defined, so it doesn't matter much, but since we have > an if statement there, let's add {} to clarify what it should do if it's > uncommented. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/omap_dma.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/hw/omap_dma.c b/hw/omap_dma.c > index 205d010..ab5e925 100644 > --- a/hw/omap_dma.c > +++ b/hw/omap_dma.c > @@ -588,7 +588,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) > #ifdef MULTI_REQ > /* TODO: should all of this only be done if dma->update, and otherwise > * inside omap_dma_transfer_generic below - check what's faster. */ > - if (dma->update) > + if (dma->update) { > #endif > > /* If the channel is element synchronized, deactivate it */ > @@ -670,6 +670,8 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) > * bits on it. */ > #ifndef MULTI_REQ > } > +#else > + } > #endif > > omap_dma_interrupts_update(s); Doesn't this look silly? Just close the brace :)
On Thu, Oct 01, 2009 at 03:05:49PM +0200, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > MULTI_REQ is never defined, so it doesn't matter much, but since we have > > an if statement there, let's add {} to clarify what it should do if it's > > uncommented. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > hw/omap_dma.c | 4 +++- > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > diff --git a/hw/omap_dma.c b/hw/omap_dma.c > > index 205d010..ab5e925 100644 > > --- a/hw/omap_dma.c > > +++ b/hw/omap_dma.c > > @@ -588,7 +588,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) > > #ifdef MULTI_REQ > > /* TODO: should all of this only be done if dma->update, and otherwise > > * inside omap_dma_transfer_generic below - check what's faster. */ > > - if (dma->update) > > + if (dma->update) { > > #endif > > > > /* If the channel is element synchronized, deactivate it */ > > @@ -670,6 +670,8 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) > > * bits on it. */ > > #ifndef MULTI_REQ > > } > > +#else > > + } > > #endif > > > > omap_dma_interrupts_update(s); > > Doesn't this look silly? Just close the brace :) This way we get more {'s than }'s, which is even more ugly. We probably should indent the text within if (dma->update) though.
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Thu, Oct 01, 2009 at 03:05:49PM +0200, Markus Armbruster wrote: >> "Michael S. Tsirkin" <mst@redhat.com> writes: >> >> > MULTI_REQ is never defined, so it doesn't matter much, but since we have >> > an if statement there, let's add {} to clarify what it should do if it's >> > uncommented. >> > >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> > --- >> > hw/omap_dma.c | 4 +++- >> > 1 files changed, 3 insertions(+), 1 deletions(-) >> > >> > diff --git a/hw/omap_dma.c b/hw/omap_dma.c >> > index 205d010..ab5e925 100644 >> > --- a/hw/omap_dma.c >> > +++ b/hw/omap_dma.c >> > @@ -588,7 +588,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) >> > #ifdef MULTI_REQ >> > /* TODO: should all of this only be done if dma->update, and otherwise >> > * inside omap_dma_transfer_generic below - check what's faster. */ >> > - if (dma->update) >> > + if (dma->update) { >> > #endif >> > >> > /* If the channel is element synchronized, deactivate it */ >> > @@ -670,6 +670,8 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) >> > * bits on it. */ >> > #ifndef MULTI_REQ >> > } >> > +#else >> > + } >> > #endif >> > >> > omap_dma_interrupts_update(s); >> >> Doesn't this look silly? Just close the brace :) > > This way we get more {'s than }'s, which is even more ugly. > We probably should indent the text within if (dma->update) though. I'd just kill the dead code, keep only the TODO comment.
diff --git a/hw/omap_dma.c b/hw/omap_dma.c index 205d010..ab5e925 100644 --- a/hw/omap_dma.c +++ b/hw/omap_dma.c @@ -588,7 +588,7 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) #ifdef MULTI_REQ /* TODO: should all of this only be done if dma->update, and otherwise * inside omap_dma_transfer_generic below - check what's faster. */ - if (dma->update) + if (dma->update) { #endif /* If the channel is element synchronized, deactivate it */ @@ -670,6 +670,8 @@ static void omap_dma_transfer_setup(struct soc_dma_ch_s *dma) * bits on it. */ #ifndef MULTI_REQ } +#else + } #endif omap_dma_interrupts_update(s);
MULTI_REQ is never defined, so it doesn't matter much, but since we have an if statement there, let's add {} to clarify what it should do if it's uncommented. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/omap_dma.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)