| Message ID | 5c4fb010.1c69fb81.94ca0.01ae@mx.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] seq: arecordmidi: Add num-events option | expand |
Hi, On 2019/01/29 10:44, pmalani@chromium.org wrote: > @@ -775,6 +780,11 @@ int main(int argc, char *argv[]) > + case 'n': > + num_events = atoi(optarg); > + if (num_events <= 0) > + fatal("num_events must be greater than 0"); > + break; We cannot distinguish two cases; 0 is given and including non-numeraical characters. Please use strtol() or so and check return value and errno for safe. For your information: http://git.alsa-project.org/?p=alsa-utils.git;a=blob;f=axfer/main.c;hb=HEAD#l44 Regards Takashi Sakamoto
Hi Takashi, On Mon, Jan 28, 2019 at 7:36 PM Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote: > Hi, > > On 2019/01/29 10:44, pmalani@chromium.org wrote: > > @@ -775,6 +780,11 @@ int main(int argc, char *argv[]) > > + case 'n': > > + num_events = atoi(optarg); > > + if (num_events <= 0) > > + fatal("num_events must be greater than 0"); > > + break; > > We cannot distinguish two cases; 0 is given and including non-numeraical > characters. Please use strtol() or so and check return value and errno > for safe. > > For your information: > > http://git.alsa-project.org/?p=alsa-utils.git;a=blob;f=axfer/main.c;hb=HEAD#l44 Can I duplicate this in arecordmidi.c ? I'm loath to introduce a header, and then refactor axfer/main.c. > > > > Regards > > Takashi Sakamoto >
Hi, On Mon, Jan 28, 2019 at 09:34:59PM -0800, Prashant Malani wrote: > Can I duplicate this in arecordmidi.c ? I'm loath to introduce a > header, and then refactor axfer/main.c. I don't mind it. When parsing string with non-numeric characters, the behaviour of atoi() is undefined still in C11. $ cd alsa-utils.git $ git grep atoi | grep optarg | wc 39 158 1919 Mmm... Many parts are written without enough care of the undefined behaviour... Thanks Takashi Sakamoto
Takashi Sakamoto wrote: > When parsing string with non-numeric characters, the behaviour of > atoi() is undefined still in C11. A completely non-numeric string is specified to return zero. The behaviour is undefined only if the value cannot be represented, i.e., if it overflows. Regards, Clemens
Interesting. Thanks for pointing that out, Clemens. In any case, I've sent both versions (atoi v/s strtol), so whichever one is preferred can be used (would be slightly inclined towards atoi, since it's used elsewhere in the file, and because of Clemens' comment; additionally, there is less code duplication with the atoi version). Best regards, On Tue, Jan 29, 2019 at 8:19 AM Clemens Ladisch <clemens@ladisch.de> wrote: > Takashi Sakamoto wrote: > > When parsing string with non-numeric characters, the behaviour of > > atoi() is undefined still in C11. > > A completely non-numeric string is specified to return zero. > > The behaviour is undefined only if the value cannot be represented, i.e., > if it overflows. > > > Regards, > Clemens >
Friendly ping. Sounds like Clemens mentioned atoi() should be OK (given his explanation regarding non-numeric strings, along with consistency with the rest of the utility). Are there any other concerns with this patchset? Thanks once again for taking the time to review. Best regards, On Tue, Jan 29, 2019 at 3:42 PM Prashant Malani <pmalani@chromium.org> wrote: > Interesting. Thanks for pointing that out, Clemens. > > In any case, I've sent both versions (atoi v/s strtol), so whichever one > is preferred can be used (would be slightly inclined towards atoi, since > it's used elsewhere in the file, and because of Clemens' comment; > additionally, there is less code duplication with the atoi version). > > Best regards, > > On Tue, Jan 29, 2019 at 8:19 AM Clemens Ladisch <clemens@ladisch.de> > wrote: > >> Takashi Sakamoto wrote: >> > When parsing string with non-numeric characters, the behaviour of >> > atoi() is undefined still in C11. >> >> A completely non-numeric string is specified to return zero. >> >> The behaviour is undefined only if the value cannot be represented, i.e., >> if it overflows. >> >> >> Regards, >> Clemens >> >
Friendly ping. On Mon, Feb 4, 2019 at 12:20 PM Prashant Malani <pmalani@chromium.org> wrote: > Friendly ping. Sounds like Clemens mentioned atoi() should be OK (given > his explanation regarding non-numeric strings, along with consistency with > the rest of the utility). > Are there any other concerns with this patchset? > > Thanks once again for taking the time to review. > > Best regards, > > On Tue, Jan 29, 2019 at 3:42 PM Prashant Malani <pmalani@chromium.org> > wrote: > >> Interesting. Thanks for pointing that out, Clemens. >> >> In any case, I've sent both versions (atoi v/s strtol), so whichever one >> is preferred can be used (would be slightly inclined towards atoi, since >> it's used elsewhere in the file, and because of Clemens' comment; >> additionally, there is less code duplication with the atoi version). >> >> Best regards, >> >> On Tue, Jan 29, 2019 at 8:19 AM Clemens Ladisch <clemens@ladisch.de> >> wrote: >> >>> Takashi Sakamoto wrote: >>> > When parsing string with non-numeric characters, the behaviour of >>> > atoi() is undefined still in C11. >>> >>> A completely non-numeric string is specified to return zero. >>> >>> The behaviour is undefined only if the value cannot be represented, i.e., >>> if it overflows. >>> >>> >>> Regards, >>> Clemens >>> >>
On Mon, 11 Feb 2019 05:23:36 +0100, Prashant Malani wrote: > > Friendly ping. Since no one raised objection, I applied now. thanks, Takashi > On Mon, Feb 4, 2019 at 12:20 PM Prashant Malani <pmalani@chromium.org> > wrote: > > > Friendly ping. Sounds like Clemens mentioned atoi() should be OK (given > > his explanation regarding non-numeric strings, along with consistency with > > the rest of the utility). > > Are there any other concerns with this patchset? > > > > Thanks once again for taking the time to review. > > > > Best regards, > > > > On Tue, Jan 29, 2019 at 3:42 PM Prashant Malani <pmalani@chromium.org> > > wrote: > > > >> Interesting. Thanks for pointing that out, Clemens. > >> > >> In any case, I've sent both versions (atoi v/s strtol), so whichever one > >> is preferred can be used (would be slightly inclined towards atoi, since > >> it's used elsewhere in the file, and because of Clemens' comment; > >> additionally, there is less code duplication with the atoi version). > >> > >> Best regards, > >> > >> On Tue, Jan 29, 2019 at 8:19 AM Clemens Ladisch <clemens@ladisch.de> > >> wrote: > >> > >>> Takashi Sakamoto wrote: > >>> > When parsing string with non-numeric characters, the behaviour of > >>> > atoi() is undefined still in C11. > >>> > >>> A completely non-numeric string is specified to return zero. > >>> > >>> The behaviour is undefined only if the value cannot be represented, i.e., > >>> if it overflows. > >>> > >>> > >>> Regards, > >>> Clemens > >>> > >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
Thanks Takashi! On Mon, Feb 11, 2019, 00:15 Takashi Iwai <tiwai@suse.de> wrote: > On Mon, 11 Feb 2019 05:23:36 +0100, > Prashant Malani wrote: > > > > Friendly ping. > > Since no one raised objection, I applied now. > > > thanks, > > Takashi > > > > On Mon, Feb 4, 2019 at 12:20 PM Prashant Malani <pmalani@chromium.org> > > wrote: > > > > > Friendly ping. Sounds like Clemens mentioned atoi() should be OK (given > > > his explanation regarding non-numeric strings, along with consistency > with > > > the rest of the utility). > > > Are there any other concerns with this patchset? > > > > > > Thanks once again for taking the time to review. > > > > > > Best regards, > > > > > > On Tue, Jan 29, 2019 at 3:42 PM Prashant Malani <pmalani@chromium.org> > > > wrote: > > > > > >> Interesting. Thanks for pointing that out, Clemens. > > >> > > >> In any case, I've sent both versions (atoi v/s strtol), so whichever > one > > >> is preferred can be used (would be slightly inclined towards atoi, > since > > >> it's used elsewhere in the file, and because of Clemens' comment; > > >> additionally, there is less code duplication with the atoi version). > > >> > > >> Best regards, > > >> > > >> On Tue, Jan 29, 2019 at 8:19 AM Clemens Ladisch <clemens@ladisch.de> > > >> wrote: > > >> > > >>> Takashi Sakamoto wrote: > > >>> > When parsing string with non-numeric characters, the behaviour of > > >>> > atoi() is undefined still in C11. > > >>> > > >>> A completely non-numeric string is specified to return zero. > > >>> > > >>> The behaviour is undefined only if the value cannot be represented, > i.e., > > >>> if it overflows. > > >>> > > >>> > > >>> Regards, > > >>> Clemens > > >>> > > >> > > _______________________________________________ > > Alsa-devel mailing list > > Alsa-devel@alsa-project.org > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > >
diff --git a/seq/aplaymidi/arecordmidi.c b/seq/aplaymidi/arecordmidi.c index 19dbb7d..ca25bfa 100644 --- a/seq/aplaymidi/arecordmidi.c +++ b/seq/aplaymidi/arecordmidi.c @@ -690,7 +690,8 @@ static void help(const char *argv0) " -t,--ticks=ticks resolution in ticks per beat or frame\n" " -s,--split-channels create a track for each channel\n" " -m,--metronome=client:port play a metronome signal\n" - " -i,--timesig=nn:dd time signature\n", + " -i,--timesig=nn:dd time signature\n" + " -n,--num-events=events fixed number of events to record, then exit\n", argv0); } @@ -706,7 +707,7 @@ static void sighandler(int sig) int main(int argc, char *argv[]) { - static const char short_options[] = "hVlp:b:f:t:sdm:i:"; + static const char short_options[] = "hVlp:b:f:t:sdm:i:n:"; static const struct option long_options[] = { {"help", 0, NULL, 'h'}, {"version", 0, NULL, 'V'}, @@ -719,6 +720,7 @@ int main(int argc, char *argv[]) {"dump", 0, NULL, 'd'}, {"metronome", 1, NULL, 'm'}, {"timesig", 1, NULL, 'i'}, + {"num-events", 1, NULL, 'n'}, { } }; @@ -727,6 +729,9 @@ int main(int argc, char *argv[]) struct pollfd *pfds; int npfds; int c, err; + /* If |num_events| isn't specified, leave it at 0. */ + int num_events = 0; + int events_received = 0; init_seq(); @@ -775,6 +780,11 @@ int main(int argc, char *argv[]) case 'i': time_signature(optarg); break; + case 'n': + num_events = atoi(optarg); + if (num_events <= 0) + fatal("num_events must be greater than 0"); + break; default: help(argv[0]); return 1; @@ -864,13 +874,20 @@ int main(int argc, char *argv[]) err = snd_seq_event_input(seq, &event); if (err < 0) break; - if (event) + if (event) { record_event(event); + events_received++; + } } while (err > 0); if (stop) break; + if (num_events && (events_received == num_events)) + break; } + if (num_events && events_received < num_events) + fputs("Warning: Received signal before num_events\n", stdout); + finish_tracks(); write_file();