Re: [PATCH v4 06/13] sim/erc32: Use gdb callback for UART I/O when linked with gdb.

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 4232 bytes --]

On 17 Mar 2015 22:02, Jiri Gaisler wrote:
> 	Use the host_callback feature for printing when linked with gdb.

please write ChangeLog entries

>      if (dumbio)
>          return; /* do nothing */
> -    if (!ifd1)
> +    if (ifd1 == 0 && f1open) {
>  	tcsetattr(0, TCSANOW, &ioc1);
> -    if (!ifd2)
> +        tcflush(ifd1, TCIFLUSH);

8 spaces -> 1 tab

> +    }
> +    if (ifd2 == 0 && f1open) {
>  	tcsetattr(0, TCSANOW, &ioc2);
> +        tcflush(ifd2, TCIFLUSH);

8 spaces -> 1 tab

> +        if (callback && !callback->isatty(callback, ifd1)) {
> +            tty_setup = 0;
> +        }

drop the braces

>  	if (sis_verbose)
>  	    printf("serial port A on stdin/stdout\n");
>          if (!dumbio) {
>              tcgetattr(ifd1, &ioc1);
> +            if (tty_setup) {
>              iocold1 = ioc1;
>              ioc1.c_lflag &= ~(ICANON | ECHO);
>              ioc1.c_cc[VMIN] = 0;
>              ioc1.c_cc[VTIME] = 0;
>          }
> +        }

need to fix indentation

> @@ -1027,17 +1042,19 @@ port_init()
>  	    printf("serial port B on stdin/stdout\n");
>          if (!dumbio) {
>              tcgetattr(ifd2, &ioc2);
> +            if (tty_setup) {
>              iocold2 = ioc2;
>              ioc2.c_lflag &= ~(ICANON | ECHO);
>              ioc2.c_cc[VMIN] = 0;
>              ioc2.c_cc[VTIME] = 0;
>          }
> +        }

need to fix indentation

> -        if (!dumbio && ofd2 == 1) setbuf(f2out, NULL);
> +	if (!dumbio && tty_setup && ofd2 == 1) setbuf(f2out, NULL);

uncuddle while you're here

> @@ -1066,6 +1083,9 @@ read_uart(addr)
>  	    if (f1open) {
>  	        anum = DO_STDIO_READ(ifd1, aq, UARTBUF);
>  	    }
> +      else {
> +          anum = 0;
> +      }

drop the braces

> @@ -1098,6 +1118,9 @@ read_uart(addr)
>  	    if (f2open) {
>  		bnum = DO_STDIO_READ(ifd2, bq, UARTBUF);
>  	    }
> +	    else {
> +		bnum = 0;
> +	    }

drop the braces

> @@ -1130,6 +1153,9 @@ read_uart(addr)
>  	    if (f1open) {
>  	        anum = DO_STDIO_READ(ifd1, aq, UARTBUF);
>              }
> +	    else {
> +		anum = 0;
> +	    }

drop the braces

> @@ -1142,6 +1168,9 @@ read_uart(addr)
>  	    if (f2open) {
>  		bnum = DO_STDIO_READ(ifd2, bq, UARTBUF);
>  	    }
> +	    else {
> +		bnum = 0;
> +	    }

drop the braces

> @@ -1182,8 +1211,12 @@ write_uart(addr, data)
>  	    if (wnuma < UARTBUF)
>  	        wbufa[wnuma++] = c;
>  	    else {
> -	        while (wnuma)
> +	        while (wnuma) {
> +              if (ofd1 == 1 && callback)
> +                  wnuma -= callback->write_stdout(callback, wbufa, wnuma);
> +              else

needs tabs for indenting


> @@ -1206,8 +1239,12 @@ write_uart(addr, data)
>  	    if (wnumb < UARTBUF)
>  		wbufb[wnumb++] = c;
>  	    else {
> -		while (wnumb)
> +          while (wnumb) {
> +              if (ofd1 == 1 && callback)
> +                  wnumb -= callback->write_stdout(callback, wbufb, wnumb);
> +              else
>  		    wnumb -= fwrite(wbufb, 1, wnumb, f2out);
> +          }

same here

> @@ -1245,19 +1282,37 @@ write_uart(addr, data)
>  static void
>  flush_uart()
>  {
> -    while (wnuma && f1open)
> +    while (wnuma && f1open) {
> +        if (ofd1 == 1 && callback) {
> +            wnuma -= callback->write_stdout(callback, wbufa, wnuma);
> +            callback->flush_stdout(callback);
> +        }
> +        else

8 spaces -> 1 tab

>  	wnuma -= fwrite(wbufa, 1, wnuma, f1out);
> -    while (wnumb && f2open)
> +    }
> +    while (wnumb && f2open) {
> +        if (ofd2 == 1 && callback) {
> +            wnuma -= callback->write_stdout(callback, wbufb, wnuma);
> +            callback->flush_stdout(callback);
> +        }
> +        else

8 spaces -> 1 tab

this keeps coming up.  i'll stop noting it with the expectation  you'll fix all 
the new code.

> +            while (callback->write_stdout(callback, &uarta_sreg, 1) != 1);

should be a space before the ( -- comes up a few times in this patch

also, prefer to have an explicit body rather than cuddle the ;
	while (...)
		continue;

comes up a few times in this patch
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]