* [alsa-devel] [PATCH 1/4] treewide: sys/poll to poll
@ 2019-11-20 4:28 Rosen Penev
2019-11-20 4:28 ` [alsa-devel] [PATCH 2/4] treewide: Fix wrong formats on 32-bit Rosen Penev
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Rosen Penev @ 2019-11-20 4:28 UTC (permalink / raw)
To: alsa-devel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [alsa-devel] [PATCH 2/4] treewide: Fix wrong formats on 32-bit 2019-11-20 4:28 [alsa-devel] [PATCH 1/4] treewide: sys/poll to poll Rosen Penev @ 2019-11-20 4:28 ` Rosen Penev 2019-11-20 4:28 ` [alsa-devel] [PATCH 3/4] treewide: Fix printf formats Rosen Penev 2019-11-20 4:28 ` [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well Rosen Penev 2 siblings, 0 replies; 9+ messages in thread From: Rosen Penev @ 2019-11-20 4:28 UTC (permalink / raw) To: alsa-devel 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [alsa-devel] [PATCH 3/4] treewide: Fix printf formats 2019-11-20 4:28 [alsa-devel] [PATCH 1/4] treewide: sys/poll to poll Rosen Penev 2019-11-20 4:28 ` [alsa-devel] [PATCH 2/4] treewide: Fix wrong formats on 32-bit Rosen Penev @ 2019-11-20 4:28 ` Rosen Penev 2019-11-20 4:28 ` [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well Rosen Penev 2 siblings, 0 replies; 9+ messages in thread From: Rosen Penev @ 2019-11-20 4:28 UTC (permalink / raw) To: alsa-devel 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well 2019-11-20 4:28 [alsa-devel] [PATCH 1/4] treewide: sys/poll to poll Rosen Penev 2019-11-20 4:28 ` [alsa-devel] [PATCH 2/4] treewide: Fix wrong formats on 32-bit Rosen Penev 2019-11-20 4:28 ` [alsa-devel] [PATCH 3/4] treewide: Fix printf formats Rosen Penev @ 2019-11-20 4:28 ` Rosen Penev 2019-11-20 6:26 ` Takashi Iwai 2 siblings, 1 reply; 9+ messages in thread From: Rosen Penev @ 2019-11-20 4:28 UTC (permalink / raw) To: alsa-devel 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well 2019-11-20 4:28 ` [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well Rosen Penev @ 2019-11-20 6:26 ` Takashi Iwai 2019-11-20 6:36 ` Rosen Penev 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2019-11-20 6:26 UTC (permalink / raw) To: Rosen Penev; +Cc: alsa-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well 2019-11-20 6:26 ` Takashi Iwai @ 2019-11-20 6:36 ` Rosen Penev 2019-11-20 6:48 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Rosen Penev @ 2019-11-20 6:36 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well 2019-11-20 6:36 ` Rosen Penev @ 2019-11-20 6:48 ` Takashi Iwai 2019-11-20 17:55 ` Rosen Penev 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2019-11-20 6:48 UTC (permalink / raw) To: Rosen Penev; +Cc: alsa-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well 2019-11-20 6:48 ` Takashi Iwai @ 2019-11-20 17:55 ` Rosen Penev [not found] ` <s5hk17uv1lm.wl-tiwai@suse.de> 0 siblings, 1 reply; 9+ messages in thread From: Rosen Penev @ 2019-11-20 17:55 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <s5hk17uv1lm.wl-tiwai@suse.de>]
* Re: [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well [not found] ` <s5hk17uv1lm.wl-tiwai@suse.de> @ 2019-11-20 19:12 ` Rosen Penev 0 siblings, 0 replies; 9+ messages in thread From: Rosen Penev @ 2019-11-20 19:12 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-20 19:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 4:28 [alsa-devel] [PATCH 1/4] treewide: sys/poll to poll Rosen Penev
2019-11-20 4:28 ` [alsa-devel] [PATCH 2/4] treewide: Fix wrong formats on 32-bit Rosen Penev
2019-11-20 4:28 ` [alsa-devel] [PATCH 3/4] treewide: Fix printf formats Rosen Penev
2019-11-20 4:28 ` [alsa-devel] [PATCH 4/4] aplay: Limit VUMeter progress bar to 100 for negative as well Rosen Penev
2019-11-20 6:26 ` Takashi Iwai
2019-11-20 6:36 ` Rosen Penev
2019-11-20 6:48 ` Takashi Iwai
2019-11-20 17:55 ` Rosen Penev
[not found] ` <s5hk17uv1lm.wl-tiwai@suse.de>
2019-11-20 19:12 ` Rosen Penev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).