RFA: parse 'target remote' device special cases first

RFA: parse 'target remote' device special cases first

[Jim Blandy]

Without this change, if the command supplied after 'target remote |'
contained a colon, the portion of the command ahead of that colon
would be mistaken for a hostname.  But the syntax isn't actually
ambiguous, since hostnames don't start with '|'.

Tested on x86_64-pc-linux-gnu.

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.

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,8 @@ serial_open (const char *name)
       ops = serial_interface_lookup ("pipe");
       open_name = name + 1; /* discard ``|'' */
     }
+  else if (strchr (name, ':'))
+    ops = serial_interface_lookup ("tcp");
   else
     ops = serial_interface_lookup ("hardwire");
 

Re: RFA: parse 'target remote' device special cases first

[Daniel Jacobowitz]

On Wed, Mar 08, 2006 at 04:12:32PM -0800, Jim Blandy wrote:
> 
> Without this change, if the command supplied after 'target remote |'
> contained a colon, the portion of the command ahead of that colon
> would be mistaken for a hostname.  But the syntax isn't actually
> ambiguous, since hostnames don't start with '|'.
> 
> Tested on x86_64-pc-linux-gnu.
> 
> 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.

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

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFA: parse 'target remote' device special cases first

[Jim Blandy]

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");

Re: RFA: parse 'target remote' device special cases first

[Daniel Jacobowitz]

On Tue, Apr 11, 2006 at 01:32:43PM -0700, Jim Blandy wrote:
> 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.

No, that seems fine to me.  Thanks.

-- 
Daniel Jacobowitz
CodeSourcery