[alsa-devel] [PATCH 1/4] treewide: sys/poll to poll

[alsa-devel] [PATCH 1/4] treewide: sys/poll to poll

[Rosen Penev]

Fixes warning on musl:

warning redirecting incorrect #include <sys/poll.h> to <poll.h>

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 amidi/amidi.c               | 2 +-
 amixer/amixer.c             | 2 +-
 aplay/aplay.c               | 2 +-
 seq/aplaymidi/arecordmidi.c | 2 +-
 seq/aseqdump/aseqdump.c     | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/amidi/amidi.c b/amidi/amidi.c
index b6e05e1..c6268e4 100644
--- a/amidi/amidi.c
+++ b/amidi/amidi.c
@@ -31,7 +31,7 @@
 #include <signal.h>
 #include <sys/timerfd.h>
 #include <sys/types.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <sys/stat.h>
 #include <unistd.h>
 #include <fcntl.h>
diff --git a/amixer/amixer.c b/amixer/amixer.c
index ad9c482..f7f31f0 100644
--- a/amixer/amixer.c
+++ b/amixer/amixer.c
@@ -28,7 +28,7 @@
 #include <errno.h>
 #include <assert.h>
 #include <alsa/asoundlib.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <stdint.h>
 #include "amixer.h"
 #include "../alsamixer/volume_mapping.h"
diff --git a/aplay/aplay.c b/aplay/aplay.c
index 274bbce..5241068 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -44,7 +44,7 @@
 #include <assert.h>
 #include <termios.h>
 #include <signal.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <sys/uio.h>
 #include <sys/time.h>
 #include <sys/stat.h>
diff --git a/seq/aplaymidi/arecordmidi.c b/seq/aplaymidi/arecordmidi.c
index f3db65e..604cd0d 100644
--- a/seq/aplaymidi/arecordmidi.c
+++ b/seq/aplaymidi/arecordmidi.c
@@ -27,7 +27,7 @@
 #include <string.h>
 #include <signal.h>
 #include <getopt.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <alsa/asoundlib.h>
 #include "aconfig.h"
 #include "version.h"
diff --git a/seq/aseqdump/aseqdump.c b/seq/aseqdump/aseqdump.c
index 7904540..578e06f 100644
--- a/seq/aseqdump/aseqdump.c
+++ b/seq/aseqdump/aseqdump.c
@@ -25,7 +25,7 @@
 #include <string.h>
 #include <signal.h>
 #include <getopt.h>
-#include <sys/poll.h>
+#include <poll.h>
 #include <alsa/asoundlib.h>
 #include "aconfig.h"
 #include "version.h"
-- 
2.23.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 2/4] treewide: Fix wrong formats on 32-bit

[Rosen Penev]

uint64_t evaluates to unsigned long long on 32-bit, not unsigned long.
Use the proper formats.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 axfer/container.c       | 7 ++++---
 axfer/subcmd-transfer.c | 4 +++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/axfer/container.c b/axfer/container.c
index 7da97c6..566acd0 100644
--- a/axfer/container.c
+++ b/axfer/container.c
@@ -13,6 +13,7 @@
 #include <errno.h>
 #include <string.h>
 #include <fcntl.h>
+#include <inttypes.h>
 
 static const char *const cntr_type_labels[] = {
 	[CONTAINER_TYPE_PARSER] = "parser",
@@ -356,10 +357,10 @@ int container_context_pre_process(struct container_context *cntr,
 		fprintf(stderr, "  frames/second: %u\n",
 			cntr->frames_per_second);
 		if (cntr->type == CONTAINER_TYPE_PARSER) {
-			fprintf(stderr, "  frames: %lu\n",
+			fprintf(stderr, "  frames: %" PRIu64 "\n",
 				*frame_count);
 		} else {
-			fprintf(stderr, "  max frames: %lu\n",
+			fprintf(stderr, "  max frames: %" PRIu64 "\n",
 				*frame_count);
 		}
 	}
@@ -427,7 +428,7 @@ int container_context_post_process(struct container_context *cntr,
 	assert(frame_count);
 
 	if (cntr->verbose && cntr->handled_byte_count > 0) {
-		fprintf(stderr, "  Handled bytes: %lu\n",
+		fprintf(stderr, "  Handled bytes: %" PRIu64 "\n",
 			cntr->handled_byte_count);
 	}
 
diff --git a/axfer/subcmd-transfer.c b/axfer/subcmd-transfer.c
index 3ca745a..8746e6f 100644
--- a/axfer/subcmd-transfer.c
+++ b/axfer/subcmd-transfer.c
@@ -11,6 +11,7 @@
 #include "misc.h"
 
 #include <signal.h>
+#include <inttypes.h>
 
 struct context {
 	struct xfer_context xfer;
@@ -389,7 +390,8 @@ static int context_process_frames(struct context *ctx,
 
 	if (!ctx->xfer.quiet) {
 		fprintf(stderr,
-			"%s: Expected %lu frames, Actual %lu frames\n",
+			"%s: Expected %" PRIu64 "frames, "
+			"Actual %" PRIu64 "frames\n",
 			snd_pcm_stream_name(direction), expected_frame_count,
 			*actual_frame_count);
 		if (ctx->interrupted) {
-- 
2.23.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 3/4] treewide: Fix printf formats

[Rosen Penev]

Found with cppcheck

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 alsactl/init_parse.c  |  4 ++--
 alsaloop/pcmjob.c     | 10 +++++-----
 amixer/amixer.c       |  6 +++---
 aplay/aplay.c         |  2 +-
 axfer/subcmd-list.c   |  2 +-
 axfer/xfer-libffado.c |  2 +-
 axfer/xfer-options.c  |  2 +-
 7 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/alsactl/init_parse.c b/alsactl/init_parse.c
index 562e674..c048fd3 100644
--- a/alsactl/init_parse.c
+++ b/alsactl/init_parse.c
@@ -187,7 +187,7 @@ static int init_space(struct space **space, int card)
 		return -ENOMEM;
 	res->ctl_id_changed = ~0;
 	res->linenum = -1;
-	sprintf(device, "hw:%u", card);
+	sprintf(device, "hw:%d", card);
 	err = snd_hctl_open(&res->ctl_handle, device, 0);
 	if (err < 0)
 		goto error;
@@ -734,7 +734,7 @@ dbvalue:
 			elem = snd_hctl_elem_next(elem);
 		}
 		snd_ctl_elem_id_free(id);
-		sprintf(res, "%u", index);
+		sprintf(res, "%d", index);
 		dbg("do_ctl_count found %s controls", res);
 		return res;
 	}
diff --git a/alsaloop/pcmjob.c b/alsaloop/pcmjob.c
index 29d1aba..b252486 100644
--- a/alsaloop/pcmjob.c
+++ b/alsaloop/pcmjob.c
@@ -120,7 +120,7 @@ static int setparams_stream(struct loopback_handle *lhandle,
 	}
 	err = snd_pcm_hw_params_set_rate_resample(handle, params, lhandle->resample);
 	if (err < 0) {
-		logit(LOG_CRIT, "Resample setup failed for %s (val %i): %s\n", lhandle->id, lhandle->resample, snd_strerror(err));
+		logit(LOG_CRIT, "Resample setup failed for %s (val %u): %s\n", lhandle->id, lhandle->resample, snd_strerror(err));
 		return err;
 	}
 	err = snd_pcm_hw_params_set_access(handle, params, lhandle->access);
@@ -135,13 +135,13 @@ static int setparams_stream(struct loopback_handle *lhandle,
 	}
 	err = snd_pcm_hw_params_set_channels(handle, params, lhandle->channels);
 	if (err < 0) {
-		logit(LOG_CRIT, "Channels count (%i) not available for %s: %s\n", lhandle->channels, lhandle->id, snd_strerror(err));
+		logit(LOG_CRIT, "Channels count (%u) not available for %s: %s\n", lhandle->channels, lhandle->id, snd_strerror(err));
 		return err;
 	}
 	rrate = lhandle->rate_req;
 	err = snd_pcm_hw_params_set_rate_near(handle, params, &rrate, 0);
 	if (err < 0) {
-		logit(LOG_CRIT, "Rate %iHz not available for %s: %s\n", lhandle->rate_req, lhandle->id, snd_strerror(err));
+		logit(LOG_CRIT, "Rate %uHz not available for %s: %s\n", lhandle->rate_req, lhandle->id, snd_strerror(err));
 		return err;
 	}
 	rrate = 0;
@@ -152,7 +152,7 @@ static int setparams_stream(struct loopback_handle *lhandle,
 	    !lhandle->loopback->src_enable &&
 #endif
 	    (int)rrate != lhandle->rate) {
-		logit(LOG_CRIT, "Rate does not match (requested %iHz, got %iHz, resample %i)\n", lhandle->rate, rrate, lhandle->resample);
+		logit(LOG_CRIT, "Rate does not match (requested %uHz, got %uHz, resample %u)\n", lhandle->rate, rrate, lhandle->resample);
 		return -EINVAL;
 	}
 	lhandle->pitch = (double)lhandle->rate_req / (double)lhandle->rate;
@@ -1613,7 +1613,7 @@ __again:
 	if (count > loop->play->buffer_size)
 		count = loop->play->buffer_size;
 	if (err != count) {
-		logit(LOG_CRIT, "%s: initial playback fill error (%i/%i/%i)\n", loop->id, err, (int)count, loop->play->buffer_size);
+		logit(LOG_CRIT, "%s: initial playback fill error (%i/%i/%u)\n", loop->id, err, (int)count, loop->play->buffer_size);
 		err = -EIO;
 		goto __error;
 	}
diff --git a/amixer/amixer.c b/amixer/amixer.c
index f7f31f0..928f7c5 100644
--- a/amixer/amixer.c
+++ b/amixer/amixer.c
@@ -465,7 +465,7 @@ static void decode_tlv(unsigned int spaces, unsigned int *tlv, unsigned int tlv_
 	size = tlv[idx++];
 	tlv_size -= 2 * sizeof(unsigned int);
 	if (size > tlv_size) {
-		printf("TLV size error (%i, %i, %i)!\n", type, size, tlv_size);
+		printf("TLV size error (%u, %u, %u)!\n", type, size, tlv_size);
 		return;
 	}
 	switch (type) {
@@ -576,7 +576,7 @@ static void decode_tlv(unsigned int spaces, unsigned int *tlv, unsigned int tlv_
 		break;
 #endif
 	default:
-		printf("unk-%i-", type);
+		printf("unk-%u-", type);
 		while (size > 0) {
 			printf("0x%08x,", tlv[idx++]);
 			size -= sizeof(unsigned int);
@@ -611,7 +611,7 @@ static int show_control(const char *space, snd_hctl_elem_t *elem,
 	}
 	count = snd_ctl_elem_info_get_count(info);
 	type = snd_ctl_elem_info_get_type(info);
-	printf("%s; type=%s,access=%s,values=%i", space, control_type(info), control_access(info), count);
+	printf("%s; type=%s,access=%s,values=%u", space, control_type(info), control_access(info), count);
 	switch (type) {
 	case SND_CTL_ELEM_TYPE_INTEGER:
 		printf(",min=%li,max=%li,step=%li\n", 
diff --git a/aplay/aplay.c b/aplay/aplay.c
index 5241068..72fa567 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -3346,7 +3346,7 @@ static void playbackv(char **names, unsigned int count)
 		}
 		alloced = 1;
 	} else if (count != channels) {
-		error(_("You need to specify %d files"), channels);
+		error(_("You need to specify %u files"), channels);
 		prg_exit(EXIT_FAILURE);
 	}
 
diff --git a/axfer/subcmd-list.c b/axfer/subcmd-list.c
index e22628c..359f73f 100644
--- a/axfer/subcmd-list.c
+++ b/axfer/subcmd-list.c
@@ -31,7 +31,7 @@ static int dump_device(snd_ctl_t *handle, const char *id, const char *name,
 	       snd_pcm_info_get_name(info));
 
 	count = snd_pcm_info_get_subdevices_count(info);
-	printf("  Subdevices: %i/%i\n",
+	printf("  Subdevices: %i/%u\n",
 	       snd_pcm_info_get_subdevices_avail(info), count);
 
 	for (i = 0; i < count; ++i) {
diff --git a/axfer/xfer-libffado.c b/axfer/xfer-libffado.c
index a37cce6..6db835d 100644
--- a/axfer/xfer-libffado.c
+++ b/axfer/xfer-libffado.c
@@ -440,7 +440,7 @@ static int xfer_libffado_pre_process(struct xfer_context *xfer,
 	}
 	if (*samples_per_frame != channels) {
 		fprintf(stderr,
-			"The number of samples per frame should be %i.\n",
+			"The number of samples per frame should be %u.\n",
 			channels);
 		return -EINVAL;
 	}
diff --git a/axfer/xfer-options.c b/axfer/xfer-options.c
index 352d126..3740b16 100644
--- a/axfer/xfer-options.c
+++ b/axfer/xfer-options.c
@@ -238,7 +238,7 @@ static int validate_options(struct xfer_context *xfer)
 		xfer->frames_per_second *= 1000;
 	if (xfer->frames_per_second < 2000 ||
 	    xfer->frames_per_second > 192000) {
-		fprintf(stderr, "bad speed value '%i'\n", val);
+		fprintf(stderr, "bad speed value '%u'\n", val);
 		return -EINVAL;
 	}
 
-- 
2.23.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

[Rosen Penev]

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]);

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 aplay/aplay.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/aplay/aplay.c b/aplay/aplay.c
index 72fa567..9c5a11b 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -54,6 +54,8 @@
 #include "formats.h"
 #include "version.h"
 
+#define ABS(a)  (a) < 0 ? -(a) : (a)
+
 #ifdef SND_CHMAP_API_VERSION
 #define CONFIG_SUPPORT_CHMAP	1
 #endif
@@ -1741,7 +1743,7 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
 			line[bar_length + 6 + 1 + p] = '+';
 		else
 			line[bar_length - p - 1] = '+';
-		if (maxperc[c] > 99)
+		if (ABS(maxperc[c]) > 99)
 			sprintf(tmp, "MAX");
 		else
 			sprintf(tmp, "%02d%%", maxperc[c]);
-- 
2.23.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

[Takashi Iwai]

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.


thanks,

Takashi

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  aplay/aplay.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index 72fa567..9c5a11b 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -54,6 +54,8 @@
>  #include "formats.h"
>  #include "version.h"
>  
> +#define ABS(a)  (a) < 0 ? -(a) : (a)
> +
>  #ifdef SND_CHMAP_API_VERSION
>  #define CONFIG_SUPPORT_CHMAP	1
>  #endif
> @@ -1741,7 +1743,7 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
>  			line[bar_length + 6 + 1 + p] = '+';
>  		else
>  			line[bar_length - p - 1] = '+';
> -		if (maxperc[c] > 99)
> +		if (ABS(maxperc[c]) > 99)
>  			sprintf(tmp, "MAX");
>  		else
>  			sprintf(tmp, "%02d%%", maxperc[c]);
> -- 
> 2.23.0
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

[Rosen Penev]

On Tue, Nov 19, 2019 at 10:26 PM Takashi Iwai <tiwai@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.
>
>
> thanks,
>
> Takashi
>
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  aplay/aplay.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > index 72fa567..9c5a11b 100644
> > --- a/aplay/aplay.c
> > +++ b/aplay/aplay.c
> > @@ -54,6 +54,8 @@
> >  #include "formats.h"
> >  #include "version.h"
> >
> > +#define ABS(a)  (a) < 0 ? -(a) : (a)
> > +
> >  #ifdef SND_CHMAP_API_VERSION
> >  #define CONFIG_SUPPORT_CHMAP 1
> >  #endif
> > @@ -1741,7 +1743,7 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
> >                       line[bar_length + 6 + 1 + p] = '+';
> >               else
> >                       line[bar_length - p - 1] = '+';
> > -             if (maxperc[c] > 99)
> > +             if (ABS(maxperc[c]) > 99)
> >                       sprintf(tmp, "MAX");
> >               else
> >                       sprintf(tmp, "%02d%%", maxperc[c]);
> > --
> > 2.23.0
> >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

[Takashi Iwai]

On Wed, 20 Nov 2019 07:36:19 +0100,
Rosen Penev wrote:
> 
> On Tue, Nov 19, 2019 at 10:26 PM Takashi Iwai <tiwai@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?


Takashi


> >
> >
> > thanks,
> >
> > Takashi
> >
> > >
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> > >  aplay/aplay.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > index 72fa567..9c5a11b 100644
> > > --- a/aplay/aplay.c
> > > +++ b/aplay/aplay.c
> > > @@ -54,6 +54,8 @@
> > >  #include "formats.h"
> > >  #include "version.h"
> > >
> > > +#define ABS(a)  (a) < 0 ? -(a) : (a)
> > > +
> > >  #ifdef SND_CHMAP_API_VERSION
> > >  #define CONFIG_SUPPORT_CHMAP 1
> > >  #endif
> > > @@ -1741,7 +1743,7 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
> > >                       line[bar_length + 6 + 1 + p] = '+';
> > >               else
> > >                       line[bar_length - p - 1] = '+';
> > > -             if (maxperc[c] > 99)
> > > +             if (ABS(maxperc[c]) > 99)
> > >                       sprintf(tmp, "MAX");
> > >               else
> > >                       sprintf(tmp, "%02d%%", maxperc[c]);
> > > --
> > > 2.23.0
> > >
> > > _______________________________________________
> > > Alsa-devel mailing list
> > > Alsa-devel@alsa-project.org
> > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > >
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

[Rosen Penev]

On Tue, Nov 19, 2019 at 10:48 PM Takashi Iwai <tiwai@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@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.

>
>
> Takashi
>
>
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > >
> > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > > ---
> > > >  aplay/aplay.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/aplay/aplay.c b/aplay/aplay.c
> > > > index 72fa567..9c5a11b 100644
> > > > --- a/aplay/aplay.c
> > > > +++ b/aplay/aplay.c
> > > > @@ -54,6 +54,8 @@
> > > >  #include "formats.h"
> > > >  #include "version.h"
> > > >
> > > > +#define ABS(a)  (a) < 0 ? -(a) : (a)
> > > > +
> > > >  #ifdef SND_CHMAP_API_VERSION
> > > >  #define CONFIG_SUPPORT_CHMAP 1
> > > >  #endif
> > > > @@ -1741,7 +1743,7 @@ static void print_vu_meter_stereo(int *perc, int *maxperc)
> > > >                       line[bar_length + 6 + 1 + p] = '+';
> > > >               else
> > > >                       line[bar_length - p - 1] = '+';
> > > > -             if (maxperc[c] > 99)
> > > > +             if (ABS(maxperc[c]) > 99)
> > > >                       sprintf(tmp, "MAX");
> > > >               else
> > > >                       sprintf(tmp, "%02d%%", maxperc[c]);
> > > > --
> > > > 2.23.0
> > > >
> > > > _______________________________________________
> > > > Alsa-devel mailing list
> > > > Alsa-devel@alsa-project.org
> > > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > > >
> >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [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@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@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@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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel