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#...

Can I duplicate this in arecordmidi.c ? I'm loath to introduce a header, and then refactor axfer/main.c.

Regards

Takashi Sakamoto

From: Prashant Malani pmalani@chromium.org

Add a command line option to automatically exit after recording a fixed number of MIDI events. This allows a program using arecordmidi to expect a MIDI file to be written automatically when the specified number of events have been received, instead of having to send a SIGINT or SIGTERM programmatically.

It also avoids the need to have the arecordmidi process running in the background, and then constantly stat the output file to check if any bytes have been written to it (this makes for less predictable and longer-running tests).

This functionality finds use in Chrome OS functional testing, since having to send SIGTERM/SIGINT programmatically and then wait for the output file adds unpredictability and delay to the tests.

The addition of this command-line option should (hopefully) not break any existing usage.

Signed-off-by: Prashant Malani pmalani@chromium.org

diff --git a/seq/aplaymidi/arecordmidi.c b/seq/aplaymidi/arecordmidi.c index 19dbb7d..c0d0569 100644 --- a/seq/aplaymidi/arecordmidi.c +++ b/seq/aplaymidi/arecordmidi.c @@ -86,6 +86,25 @@ static int ts_num = 4; /* time signature: numerator */ static int ts_div = 4; /* time signature: denominator */ static int ts_dd = 2; /* time signature: denominator as a power of two */

+/* Parse a decimal number from a command line argument. */ +static long arg_parse_decimal_num(const char *str, int *err) +{ + long val; + char *endptr; + + errno = 0; + val = strtol(str, &endptr, 0); + if (errno > 0) { + *err = -errno; + return 0; + } + if (*endptr != '\0') { + *err = -EINVAL; + return 0; + } + + return val; +}

/* prints an error message to stderr, and dies */ static void fatal(const char *msg, ...) @@ -690,7 +709,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 +726,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 +739,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 +748,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. */ + long num_events = 0; + long events_received = 0;

init_seq();

@@ -775,6 +799,16 @@ int main(int argc, char *argv[]) case 'i': time_signature(optarg); break; + case 'n': + err = 0; + num_events = arg_parse_decimal_num(optarg, &err); + if (err != 0) { + fatal("Couldn't parse num_events argument: %s\n", + strerror(-err)); + } + if (num_events <= 0) + fatal("num_events must be greater than 0"); + break; default: help(argv[0]); return 1; @@ -864,13 +898,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();