From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Tue, 5 Mar 2019 19:43:53 +0100
Subject: [U-Boot] [PATCH v2 1/2] efi_loader: Fix serial console size
	detection
In-Reply-To: <20190305115019.3581-1-matthias.bgg@kernel.org>
References: <20190305115019.3581-1-matthias.bgg@kernel.org>
Message-ID: <82befdbd-4e87-b593-d768-07e046ed30e2@gmx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 3/5/19 12:50 PM, matthias.bgg at kernel.org wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> Function term_read_reply tries to read from the serial console until
> the end_char was read. This can hang forever if we are, for some reason,
> not be able to read the full response (e.g. serial buffer too small,
> frame error). This patch moves the timeout detection into
> term_read_reply to assure we will make progress.
> 
> Fixes: 6bb591f704 ("efi_loader: query serial console size reliably")
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> 
> ---
> 
> Changes in v2:
> - move timeout into term_get_char
> 
>  lib/efi_loader/efi_console.c | 60 ++++++++++++++++++++----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 66c33a551d..1133674faf 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -62,6 +62,21 @@ static struct simple_text_output_mode efi_con_mode = {
>  	.cursor_visible = 1,
>  };
>  
> +static int term_get_char(char *c)
> +{
> +	u64 timeout;
> +
> +	/* Wait up to 100 ms for a character */
> +	timeout = timer_get_us() + 100000;
> +
> +	while (!tstc())
> +		if (timer_get_us() > timeout)
> +			return 1;
> +
> +	*c = getc();
> +	return 0;
> +}
> +
>  /*
>   * Receive and parse a reply from the terminal.
>   *
> @@ -75,31 +90,31 @@ static int term_read_reply(int *n, int num, char end_char)
>  	char c;
>  	int i = 0;
>  
> -	c = getc();
> -	if (c != cESC)
> +	if (term_get_char(&c) || c != cESC)
>  		return -1;
> -	c = getc();
> -	if (c != '[')
> +
> +	if (term_get_char(&c) || c != '[')
>  		return -1;
>  
>  	n[0] = 0;
>  	while (1) {
> -		c = getc();
> -		if (c == ';') {
> -			i++;
> -			if (i >= num)
> +		if (!term_get_char(&c)) {
> +			if (c == ';') {
> +				i++;
> +				if (i >= num)
> +					return -1;
> +				n[i] = 0;
> +				continue;
> +			} else if (c == end_char) {
> +				break;
> +			} else if (c > '9' || c < '0') {
>  				return -1;
> -			n[i] = 0;
> -			continue;
> -		} else if (c == end_char) {
> -			break;
> -		} else if (c > '9' || c < '0') {
> -			return -1;
> -		}
> +			}
>  
> -		/* Read one more decimal position */
> -		n[i] *= 10;
> -		n[i] += c - '0';
> +			/* Read one more decimal position */
> +			n[i] *= 10;
> +			n[i] += c - '0';
> +		}

If no character is received we should throw an error:

	} else {
		return -1;

Best regards

Heinrich

>  	}
>  	if (i != num - 1)
>  		return -1;
> @@ -196,7 +211,6 @@ static int query_console_serial(int *rows, int *cols)
>  {
>  	int ret = 0;
>  	int n[2];
> -	u64 timeout;
>  
>  	/* Empty input buffer */
>  	while (tstc())
> @@ -216,14 +230,6 @@ static int query_console_serial(int *rows, int *cols)
>  	       ESC "[999;999H"	/* Move to bottom right corner */
>  	       ESC "[6n");	/* Query cursor position */
>  
> -	/* Allow up to one second for a response */
> -	timeout = timer_get_us() + 1000000;
> -	while (!tstc())
> -		if (timer_get_us() > timeout) {
> -			ret = -1;
> -			goto out;
> -		}
> -
>  	/* Read {rows,cols} */
>  	if (term_read_reply(n, 2, 'R')) {
>  		ret = 1;
>