From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2887 invoked by alias); 23 Mar 2015 13:11:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2874 invoked by uid 89); 23 Mar 2015 13:11:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 23 Mar 2015 13:11:53 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 7CFBAD356F; Mon, 23 Mar 2015 09:11:49 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id CK3ca+fliBmV; Mon, 23 Mar 2015 09:11:49 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 47661D2ED7; Mon, 23 Mar 2015 09:11:49 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 90AC540EAD; Mon, 23 Mar 2015 06:11:47 -0700 (PDT) Date: Mon, 23 Mar 2015 13:11:00 -0000 From: Joel Brobecker To: Yurij Grechishhev Cc: gdb-patches@sourceware.org Subject: Re: Setting parity for remote serial Message-ID: <20150323131147.GD5438@adacore.com> References: <20130926165939.GA5440@adacore.com> <20131004073447.GB3326@adacore.com> <20150225151646.GA30181@adacore.com> <20150227081642.GE31815@adacore.com> <20150317145620.GB7494@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2015-03/txt/msg00717.txt.bz2 > Unfortunately, now I don't have the ability to fully-test it with > windows (but I was checking initial version one year ago, with real > hardware). Now it has been compiled by using mingw with > --host=i586-mingw32msvc. No problem - the code looked about right to me, and we will let those who do work on Windows take care of fixing any bugs if they use this feature and encounter a bug. > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index ca8bbaf..02f9ddc 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,37 @@ > +2015-03-21 Yurij Grechishhev > + > + * NEWS: Mention set/show serial parity command. > + * monitor.c (monitor_open): Call serial_setparity. > + * remote.c (remote_open_1): Likewise. > + * ser-base.c (ser_base_serparity): New function. > + * ser-base.h (ser_base_setparity): Add declaration. > + * ser-go32.c (dos_ops): Add dos_noop field. The name of the struct field is actually "setparity" (instead of "dos_noop"). > + * ser-mingw.c (ser_windows_raw): Remove state.fParity and > + state.Parity definitions. "Do not set state.fParity and state.Parity". You remmoved assignments, rather than definitions. > + (ser_windows_setparity): New function. > + (hardwire_ops): Add ser_windows_setparity. > + (tty_ops): Add NULL for setparity field. > + (pipe_ops): Add ser_base_setparity. > + (tcp_ops): Likewise. > + * ser-pipe.c (pipe_ops): Likewise. > + * ser-tcp.c (tcp_ops): Likewise. > + * ser-unix.c (hardwire_setparity): Add declaration. > + (hardwire_raw): Don't reset PARENB flag. > + (hardwire_setparity): New function. > + (hardwire_ops): Add hardwire_setparity. > + * serial.c (serial_setparity): New function. > + (serial_parity): New global. > + (parity_none, parity_odd, parity_even, parity_enums, parity): > + New static globals. Wrong indentation of "New static globals". I suspect that this is because you used 8 spaces instead of a tab to indent that line? > + (set_parity): New function. > + (_initialize_serial): Add set/show serial parity commands. > + * serial.h (GDBPARITY_NONE): Define. > + (GDBPARITY_ODD): Define. > + (GDBPARITY_EVEN): Define. > + (serial_setparity) Add declaration. > + (serial_ops): Add setparity entry. Let's say... (struct serial_ops): Add setparity field ... instead. > + * target.h (serial_parity): Add declaration. The rest are minor comments: > + internal_warning (__FILE__, __LINE__, "Incorrect parity value: > %d", parity); This line is too long. It shouldn't exceed 79 characters. internal_warning (__FILE__, __LINE__, "Incorrect parity value: %d", parity); Use instead: internal_warning (__FILE__, __LINE__, "Incorrect parity value: %d", parity); Also, watch out for the fact that your mailer appears to have wrapped the line. I suggest you switch to sending the patch either using "git send-email" (the recommended method), or else sending the patch as an attachment ("git format-patch" + attach to email). > @@ -409,7 +410,7 @@ hardwire_raw (struct serial *scb) > state.termios.c_iflag = 0; > state.termios.c_oflag = 0; > state.termios.c_lflag = 0; > - state.termios.c_cflag &= ~(CSIZE | PARENB); > + state.termios.c_cflag &= ~(CSIZE); You don't really need the parentheses anymore -> ~CSIZE > state.termios.c_cflag |= CLOCAL | CS8; > #ifdef CRTSCTS > /* h/w flow control. */ > @@ -432,7 +433,7 @@ hardwire_raw (struct serial *scb) > state.termio.c_iflag = 0; > state.termio.c_oflag = 0; > state.termio.c_lflag = 0; > - state.termio.c_cflag &= ~(CSIZE | PARENB); > + state.termio.c_cflag &= ~(CSIZE); Likewise. > @@ -893,6 +894,50 @@ hardwire_setstopbits (struct serial *scb, int num) > return set_tty_state (scb, &state); > } > > +/* Implement the "setparity" serial_ops callback. */ > + > +static int > +hardwire_setparity (struct serial *scb, int parity) > +{ > + struct hardwire_ttystate state; > + int newparity = 0; > + > + if (get_tty_state (scb, &state)) > + return -1; > + > + switch (parity) > + { > + case GDBPARITY_NONE: > + newparity = 0; > + break; > + case GDBPARITY_ODD: > + newparity = PARENB | PARODD; > + break; > + case GDBPARITY_EVEN: > + newparity = PARENB; > + break; > + default: > + internal_warning (__FILE__, __LINE__, "Incorrect parity value: > %d", parity); Same as above - line is too long. > +/* See serial.h. */ > + > +int > +serial_setparity (struct serial *scb, int parity) > +{ > + return scb->ops->setparity (scb, parity); The indentation is wrong (we use a 2-space indentation). Hence: int serial_setparity (struct serial *scb, int parity) { return scb->ops->setparity (scb, parity); } -- Joel