public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v2] input/input06: Ignore auto-repeat config events
Date: Wed, 23 Aug 2017 12:49:29 +0200	[thread overview]
Message-ID: <20170823104928.GA19747@rei.lan> (raw)
In-Reply-To: <20170822220817.14088-1-sspatil@google.com>

Hi!
> The test currently fails on some android devices because the kernel
> sends the EV_REP event with the codes REP_DELAY and REP_PERIOD and
> their corresponding values for the uinput device the test creates.

Minor note, the patch has coding style violations, we do follow LKML
coding style in LTP. You can use checkpatch.pl (from kernel source tree)
to check for these.

> Signed-off-by: Sandeep Patil <sspatil@google.com>
> ---
>  testcases/kernel/input/input06.c | 118 ++++++++++++++++++++++++++++++---------
>  1 file changed, 91 insertions(+), 27 deletions(-)
> 
> diff --git a/testcases/kernel/input/input06.c b/testcases/kernel/input/input06.c
> index c4fc1ef57..962546ead 100644
> --- a/testcases/kernel/input/input06.c
> +++ b/testcases/kernel/input/input06.c
> @@ -37,6 +37,9 @@ static void cleanup(void);
>  
>  static int fd;
>  static int fd2;
> +struct input_event events[64];
> +static int num_events;
> +static int ev_iter;
>  
>  char *TCID = "input06";
>  
> @@ -107,9 +110,9 @@ static int check_event(struct input_event *iev, int event, int code, int value)
>  	return iev->type == event && iev->code == code && iev->value == value;
>  }
>  
> -static int check_bound(unsigned int i, unsigned int rd)
> +static int check_config_event(struct input_event *iev, int event, int code)
>  {
> -	return i <= rd / sizeof(struct input_event);
> +	return iev->type == event && iev->code == code;
>  }
>  
>  static void check_size(int rd)
> @@ -123,43 +126,104 @@ static void check_size(int rd)
>  	}
>  }
>  
> -static int check_events(void)
> +static void read_events(void)
>  {
> -	struct input_event iev[64];
> -	unsigned int i;
> -	int nb;
> -	int rd;
> +	int rd = read(fd2, events, sizeof(events));
>  
> -	i = 0;
> -	nb = 0;
> +	check_size(rd);

The check_size() seems to be called only from this place now, hence
there is no need to keep these two if conditions in a separate function
anymore.

> -	rd = read(fd2, iev, sizeof(iev));
> +	ev_iter = 0;
> +	num_events = rd / sizeof(struct input_event);
> +}
>  
> -	check_size(rd);
> +static int have_events(void)
> +{
> +	return num_events !=0 && ev_iter < num_events;
> +}
>  
> -	if (rd > 0 && check_event(&iev[i], EV_KEY, KEY_X, 1))
> -		i++;
> +static struct input_event *next_event()
> +{
> +	struct input_event *ev = NULL;
>  
> -	while (check_bound(i, rd) && !check_event(&iev[i], EV_KEY, KEY_X, 0)) {
> +	if (!have_events()) {
> +		read_events();
> +	}
>  
> -		if (iev[i].type != EV_SYN
> -			&& !check_event(&iev[i], EV_KEY, KEY_X, 2)) {
> -			tst_resm(TINFO,
> -				"Didn't receive EV_KEY KEY_X with value 2");
> +	ev = &events[ev_iter++];
> +
> +	return ev;
> +}
> +
> +static int check_sync_event(void) {
> +	return check_event(next_event(), EV_SYN, 0, 0);
> +}
> +
> +static int parse_autorepeat_config(struct input_event *iev)
> +{
> +	if (!check_config_event(iev, EV_REP, REP_DELAY)) {
> +		tst_resm(TFAIL,
> +			"Didn't get EV_REP configuration with code REP_DELAY");
> +		return 0;
> +	}
> +
> +	if (!check_config_event(next_event(), EV_REP, REP_PERIOD)) {
> +		tst_resm(TFAIL,
> +			"Didn't get EV_REP configuration with code REP_PERIOD");
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int parse_key(struct input_event *iev)
> +{
> +	if (!check_event(iev, EV_KEY, KEY_X, 1) || !check_sync_event()) {
> +		tst_resm(TFAIL, "Didn't get expected key press for KEY_X");
> +		return 0;
> +	}
> +
> +	iev = next_event();
> +	while (check_event(iev, EV_KEY, KEY_X, 2))
> +		iev = next_event();

I guess that we should check here that we got at least one autorepeat,
since as it is the test would pass even for just key_down + key_up
events.

> +	if (!check_event(iev, EV_KEY, KEY_X, 0) || !check_sync_event()) {
> +		tst_resm(TFAIL,
> +			"Didn't get expected key release for KEY_X");
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int check_events(void)
> +{
> +	struct input_event *iev;
> +	int ret;
> +
> +	read_events();
> +
> +	while (have_events()) {
> +		iev = next_event();
> +		switch (iev->type) {
> +		case EV_REP:
> +			ret = parse_autorepeat_config(iev);
> +			break;
> +		case EV_KEY:
> +			ret = parse_key(iev);
> +			break;

Here as well, we should make sure that we got here once before we exit
the function.

So we should add a counter here and check that it's value == 1 before
we exit this function.

> +		default:
> +			tst_resm(TFAIL,
> +				"Unexpected event type '0x%04x' received",
> +				iev->type);
> +			ret = 0;
>  			break;
>  		}
> -		i++;
> -		nb++;
>  
> -		if (i == rd / sizeof(struct input_event)) {
> -			i = 0;
> -			rd = read(fd2, iev, sizeof(iev));
> -			check_size(rd);
> -		}
> +		if (!ret)
> +			break;
>  	}
>  
> -	return (nb > 0 && check_bound(i, rd)
> -		&& check_event(&iev[i], EV_KEY, KEY_X, 0));
> +	return ret;
>  }
>  
>  static void cleanup(void)
> -- 
> 2.14.1.480.gb18f417b89-goog
> 

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2017-08-23 10:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-22 22:08 [LTP] [PATCH v2] input/input06: Ignore auto-repeat config events Sandeep Patil
2017-08-23 10:49 ` Cyril Hrubis [this message]
2017-08-24 18:03   ` Sandeep Patil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170823104928.GA19747@rei.lan \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox