Re: [PATCH qemu] timer/i8254: Fix one shot PIT mode

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 01:11:19PM +0100, BALATON Zoltan wrote:
> On Sun, 26 Feb 2023, Max Filippov wrote:
> > On Sun, Feb 26, 2023 at 1:18 AM Damien Zammit <damien@zamaudio.com> wrote:
> > > 
> > > Hi Michael,
> > > 
> > > Thanks for reviewing this on a weekend!
> > > 
> > > On 26/2/23 19:51, Michael S. Tsirkin wrote:
> > > > On Sun, Feb 26, 2023 at 01:58:10AM +0000, Damien Zammit wrote:
> > > > >       case 0:
> > > > > -        out = (d >= s->count);
> > > > > -        break;
> > > > 
> > > > 
> > > > I think you need something like
> > > >       /* FALLTHRU */
> > > > here otherwise some gcc versions will warn.
> > > > 
> > > > >       case 1:
> > > > > -        out = (d < s->count);
> > > > > +        out = (d >= s->count);
> > > 
> > > It seems that there are quite a number of these consecutive fallthrough 
> > > cases
> > > without /* FALLTHRU */ in i8254_common.c
> > > 
> > > Can these be fixed in a separate patch?
> > 
> > I believe that the comment is only needed when there's code
> > between the labels and is not needed between the labels that
> > follow each other.
> 
> I think so too, I have some of these consecutive case labels in my code and
> never had a problem with that. Only when you have a statement between labels
> without break is when a comment is needed.
> 
> Regards,
> BALATON Zoltan


I just tried and it looks like you are right. Pls ignore sorry about the
noise.

-- 
MST