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

Mike Frysinger
2015-03-28T07:13:47Z

[-- 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 --]