All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 2/2] pty04: Retry reads when short
Date: Tue, 12 May 2020 17:44:09 +0200	[thread overview]
Message-ID: <20200512154409.GA9960@yuki.lan> (raw)
In-Reply-To: <20200512142824.13063-2-rpalethorpe@suse.com>

Hi!
> +static ssize_t try_read(int fd, char *data, ssize_t size, ssize_t *n)
> +{
> +	ssize_t ret = read(fd, data, size);
> +
> +	if (ret < 0)
> +		return -(errno != EAGAIN);
> +
> +	return (*n += ret) >= size;
> +}

I had to read this piece twice, but I think it's correct.

>  static void read_netdev(const struct ldisc_info *ldisc)
>  {
> -	int rlen, plen = 0;
> +	int ret, rlen, plen = 0;
> +	ssize_t n;
>  	char *data;
>  
>  	switch (ldisc->n) {
> @@ -305,20 +316,27 @@ static void read_netdev(const struct ldisc_info *ldisc)
>  
>  	tst_res(TINFO, "Reading from socket %d", sk);
>  
> -	SAFE_READ(1, sk, data, plen);
> +	n = 0;
> +	ret = TST_RETRY_FUNC(try_read(sk, data, plen, &n), TST_RETVAL_NOTNULL);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "Read %zd of %d bytes", n, plen);

I wonder if a simple loop without exponential backoff would suffice
here. A least the code would probably be more readable.

>  	check_data(ldisc, data, plen);
>  	tst_res(TPASS, "Read netdev 1");
>  
> -	SAFE_READ(1, sk, data, plen);
> +	n = 0;
> +	ret = TST_RETRY_FUNC(try_read(sk, data, plen, &n), TST_RETVAL_NOTNULL);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "Read %zd of %d bytes", n, plen);
>  	check_data(ldisc, data, plen);
>  	tst_res(TPASS, "Read netdev 2");
>  
>  	TST_CHECKPOINT_WAKE(0);
> -	while((rlen = read(sk, data, plen)) > 0)
> +	while ((rlen = read(sk, data, plen)) > 0)
>  		check_data(ldisc, data, rlen);

This should have been part of the previous cleanup patch.

>  	tst_res(TPASS, "Reading data from netdev interrupted by hangup");
>  
> +	close(sk);
>  	tst_free_all();
>  }
>  
> @@ -357,6 +375,7 @@ static void cleanup(void)
>  {
>  	ioctl(pts, TIOCVHANGUP);
>  	ioctl(ptmx, TIOCVHANGUP);
> +	close(sk);
>  
>  	tst_reap_children();
>  }
> -- 
> 2.26.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2020-05-12 15:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-12 14:28 [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Richard Palethorpe
2020-05-12 14:28 ` [LTP] [PATCH 2/2] pty04: Retry reads when short Richard Palethorpe
2020-05-12 15:44   ` Cyril Hrubis [this message]
2020-05-12 15:49   ` Petr Vorel
2020-05-12 19:44     ` Jan Stancek
2020-05-13  8:23 ` [LTP] [PATCH 1/2] pty04: Remove unnecessary volatile and style fix Petr Vorel

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=20200512154409.GA9960@yuki.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.