Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Jim Blandy" <jimb@red-bean.com>
To: "Jim Blandy" <jimb@red-bean.com>, gdb-patches@sources.redhat.com
Subject: Re: RFA: parse 'target remote' device special cases first
Date: Tue, 11 Apr 2006 20:32:00 -0000	[thread overview]
Message-ID: <8f2776cb0604111332n5f0f665ala64bc30710ca74de@mail.gmail.com> (raw)
In-Reply-To: <20060325070020.GK26748@nevyn.them.org>

On 3/25/06, Daniel Jacobowitz <drow@false.org> wrote:
> On Wed, Mar 08, 2006 at 04:12:32PM -0800, Jim Blandy wrote:
> > src/gdb/ChangeLog:
> > 2006-03-08  Jim Blandy  <jimb@red-bean.com>
> >
> >       * serial.c (serial_open): Check for special cases at the front of
> >       the "device" name before scanning for the ':' that would indicate
> >       an IP-based connection.
>
> Doesn't this go against the "what not why" ChangeLog convention?  It
> ought to go in serial_open as a comment, I think.

You're right, there should be a comment in serial_open; I've added
one.  But the ChangeLog entry seems okay to me: it doesn't talk about
one check inadvertently masking the others, or how text following a
"|" (say) might contain colons, or things like that; it just says what
the code has been changed to do when.

> Other than that, I agree; this patch is OK.

I've committed the patch below, but I'm happy to revise the log entry
if you still think it's not of the right form.  I agree the principle
you're citing is important; it seems to me the entry adheres to it.

src/gdb/ChangeLog:
2006-04-11  Jim Blandy  <jimb@codesourcery.com>

	* serial.c (serial_open): Check for special cases at the front of
	the "device" name before scanning for the ':' that would indicate
	an IP-based connection.

Index: src/gdb/serial.c
===================================================================
--- src.orig/gdb/serial.c
+++ src/gdb/serial.c
@@ -184,8 +184,6 @@ serial_open (const char *name)

   if (strcmp (name, "pc") == 0)
     ops = serial_interface_lookup ("pc");
-  else if (strchr (name, ':'))
-    ops = serial_interface_lookup ("tcp");
   else if (strncmp (name, "lpt", 3) == 0)
     ops = serial_interface_lookup ("parallel");
   else if (strncmp (name, "|", 1) == 0)
@@ -193,6 +191,11 @@ serial_open (const char *name)
       ops = serial_interface_lookup ("pipe");
       open_name = name + 1; /* discard ``|'' */
     }
+  /* Check for a colon, suggesting an IP address/port pair.
+     Do this *after* checking for all the interesting prefixes.  We
+     don't want to constrain the syntax of what can follow them.  */
+  else if (strchr (name, ':'))
+    ops = serial_interface_lookup ("tcp");
   else
     ops = serial_interface_lookup ("hardwire");


  reply	other threads:[~2006-04-11 20:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-09  4:38 Jim Blandy
2006-03-25  7:18 ` Daniel Jacobowitz
2006-04-11 20:32   ` Jim Blandy [this message]
2006-04-11 20:42     ` Daniel Jacobowitz

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=8f2776cb0604111332n5f0f665ala64bc30710ca74de@mail.gmail.com \
    --to=jimb@red-bean.com \
    --cc=gdb-patches@sources.redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox