[PATCH evemu 2/3] evemu-record: add support for --autorestart to cycle buffers
Peter Hutterer
peter.hutterer at who-t.net
Tue Mar 1 00:45:51 UTC 2016
On Mon, Feb 29, 2016 at 10:39:32AM +0100, Benjamin Tissoires wrote:
> On Fri, Feb 26, 2016 at 3:53 AM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > Uses the existing evemu timeout feature where evemu_record() quits after a
> > given timeout. A call of
> >
> > evemu-record --autorestart 10 /dev/input/event4 scroll.evemu
> >
> > will use the filename as prefix and create a timestamped output file (e.g.
> > scroll.evemu.2016-02-25-09:13). Until the 10s inactivity timeout is hit, all
> > events are recorded to that file. After that, the file is closed with a note
> > at the bottom and a new file is started.
> >
> > This enables a user to leave evemu running in the background for a prolonged
> > time and recover the most recent recording after triggering a bug without
> > having to wade through three days of recordings.
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=93752
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > tools/evemu-record.c | 194 +++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 172 insertions(+), 22 deletions(-)
> >
> > diff --git a/tools/evemu-record.c b/tools/evemu-record.c
> > index bedb659..8a78cd1 100644
> > --- a/tools/evemu-record.c
> > +++ b/tools/evemu-record.c
[...]
> > +
> > + fprintf(output, "################################\n");
> > + fprintf(output, "# Waiting for events #\n");
> > + fprintf(output, "################################\n");
> > + if (autorestart)
> > + fprintf(output, "# Autorestart timeout: %d\n", timeout);
> > +
> > + if (evemu_record(output, fd, timeout)) {
> > + fprintf(stderr, "error: could not record device\n");
> > + } else if (autorestart) {
> > + fprintf(output, "# Closing after %ds inactivity\n",
> > + timeout/1000);
>
> It might be interesting to dump something to the stderr here too (or not).
I think it's better to not print anything here. with a low enough inactivity
timeout (which is ideal to keep short recordings) this would cause a lot of
spam for the long-running use-case we really care about. And I expect that
the interesting data is always going to be in the last or second-to-last
recording anyway.
Cheers,
Peter
> > + }
> > +
> > + fflush(output);
> > + if (output != stdout) {
> > + fclose(output);
> > + output = stdout;
> > + }
> > + } while (autorestart);
> > +
> > + rc = true;
> > +
> > +out:
> > + free(filename);
> > + return rc;
> > }
> >
> > static inline bool test_grab_device(int fd)
> > @@ -102,6 +220,10 @@ enum mode {
> > EVEMU_DESCRIBE
> > };
> >
> > +enum options {
> > + OPT_AUTORESTART,
> > +};
> > +
> > int main(int argc, char *argv[])
> > {
> > enum mode mode = EVEMU_RECORD;
> > @@ -109,6 +231,12 @@ int main(int argc, char *argv[])
> > struct sigaction act;
> > char *prgm_name = program_invocation_short_name;
> > char *device = NULL;
> > + int timeout = INFINITE;
> > + struct option opts[] = {
> > + { "autorestart", required_argument, 0, OPT_AUTORESTART },
> > + { 0, 0, 0, 0},
> > + };
> > + const char *prefix = NULL;
>
> I don't think it's good to initialize prefix here given the rest...
>
> > int rc = 1;
> >
> > output = stdout;
> > @@ -118,10 +246,34 @@ int main(int argc, char *argv[])
> > strcmp(prgm_name, "lt-evemu-describe") == 0))
> > mode = EVEMU_DESCRIBE;
> >
> > - device = (argc < 2) ? find_event_devices() : strdup(argv[1]);
> > + while (1) {
> > + int c;
> > + int option_index = 0;
> > +
> > + c = getopt_long(argc, argv, "", opts, &option_index);
> > + if (c == -1)
> > + break;
> > +
> > + switch (c) {
> > + case OPT_AUTORESTART:
> > + if (!safe_atoi(optarg, &timeout) ||
> > + timeout <= 0) {
> > + usage();
> > + goto out;
> > + }
> > + timeout *= 1000; /* sec to ms */
> > + autorestart = true;
> > + break;
> > + default:
> > + usage();
> > + goto out;
> > + }
> > + }
> > +
> > + device = (optind >= argc) ? find_event_devices() : strdup(argv[optind++]);
> >
> > if (device == NULL) {
> > - fprintf(stderr, "Usage: %s <device> [output file]\n", argv[0]);
> > + usage();
> > goto out;
> > }
> > fd = open(device, O_RDONLY | O_NONBLOCK);
> > @@ -142,18 +294,14 @@ int main(int argc, char *argv[])
> > goto out;
> > }
> >
> > - if (argc < 3)
> > - output = stdout;
> > - else {
> > - output = fopen(argv[2], "w");
> > - if (!output) {
> > - fprintf(stderr, "error: could not open output file (%m)");
> > + if (optind >= argc) {
> > + prefix = NULL;
>
> ... or do not set it to NULL here.
>
> > + if (autorestart) {
> > + fprintf(stderr, "Option --autoresume requires an output file\n");
> > + goto out;
> > }
> > - }
> > -
> > - if (describe_device(output, fd)) {
> > - fprintf(stderr, "error: could not describe device\n");
> > - goto out;
> > + } else {
> > + prefix = argv[optind++];
> > }
>
> prefix contains here argv[optind++]; ...
>
> >
> > if (mode == EVEMU_RECORD) {
> > @@ -164,17 +312,19 @@ int main(int argc, char *argv[])
> > if (!test_grab_device(fd))
> > goto out;
> >
> > - if (describe_device(output, fd)) {
> > - fprintf(stderr, "error: could not describe device\n");
> > - goto out;
> > - }
> > + record_device(fd, timeout, prefix);
> >
> > - fprintf(output, "################################\n");
> > - fprintf(output, "# Waiting for events #\n");
> > - fprintf(output, "################################\n");
> > - if (evemu_record(output, fd, INFINITE))
> > - fprintf(stderr, "error: could not record device\n");
> > } else if (mode == EVEMU_DESCRIBE) {
> > + if (optind >= argc) {
>
> This test is always true in the regular case (no extra parameter)
>
> > + output = stdout;
> > + } else {
> > + output = fopen(argv[optind++], "w");
>
> ... which should be used here. Otherwise, this breaks the existing
> evemu-describe behavior.
>
> > + if (!output) {
> > + fprintf(stderr, "error: could not open output file (%m)");
> > + goto out;
> > + }
> > + }
> > +
> > if (describe_device(output, fd)) {
> > fprintf(stderr, "error: could not describe device\n");
> > goto out;
> > --
> > 2.5.0
> >
>
> Cheers,
> Benjamin
More information about the Input-tools
mailing list