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

Jim Blandy
2006-04-11T20:32:52Z

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