[alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well

[Rosen Penev]

On Wed, Nov 20, 2019 at 10:34 AM Takashi Iwai <tiwai at suse.de> wrote:
>
> On Wed, 20 Nov 2019 18:55:43 +0100,
> Rosen Penev wrote:
> >
> > On Tue, Nov 19, 2019 at 10:48 PM Takashi Iwai <tiwai at suse.de> wrote:
> > >
> > > On Wed, 20 Nov 2019 07:36:19 +0100,
> > > Rosen Penev wrote:
> > > >
> > > > On Tue, Nov 19, 2019 at 10:26 PM Takashi Iwai <tiwai at suse.de> wrote:
> > > > >
> > > > > On Wed, 20 Nov 2019 05:28:56 +0100,
> > > > > Rosen Penev wrote:
> > > > > >
> > > > > > If the progress bar somehow becomes negative, it ends up overwritting
> > > > > > tmp. Fixes this GCC warning:
> > > > > >
> > > > > > aplay.c:1747:18: warning: '%02d' directive writing between 2 and 11 bytes
> > > > > >  into a region of size 4 [-Wformat-overflow=]
> > > > > >  1747 |    sprintf(tmp, "%02d%%", maxperc[c]);
> > > > >
> > > > > This is false-positive.  The value passed there is guaranteed to be a
> > > > > positive integer at the calculation time.
> > > > Sure. But best to silence GCC. It probably optimizes better this way.
> > >
> > > I guess this adds more code in binary.  Comparing with 99U would work?
> > I tried that. Here are some results:
> >
> > 134348 for this patch
> > 134832 if changed to U. Also tried casting lhs to unnsigned int, same size.
> > 135125 originally
> >
> > I understand this is an exercise in micro-optimization, but still.
> >
> > Sizes are in bytes. It's the size of a compressed OpenWrt archive.
>
> Thanks for the analysis.  It's surprising, though, the original code
> became bigger.
I've learned not to question the compiler. If it complains, it means
it generates suboptimal code.
>
> The cast of lhs is basically superfluous since C performs the cast
> implicitly at comparison, BTW.
>
> In anyway, the number tells.  Could you respin this patch?
I can resend. Not sure what you really want.
> Meanwhile I'll apply the rest patches.
>
>
> thanks,
>
> Takashi