[PATCH 3/3] serial: Improve detection of duplicate serial ports

[Benjamin Herrenschmidt]

We currently rely on some pretty fragile comparison by name to
identify wether a serial port being configured is identical
---
 grub-core/term/arc/serial.c      |  4 +++
 grub-core/term/ieee1275/serial.c |  4 +++
 grub-core/term/ns8250.c          | 16 +++++++++
 grub-core/term/serial.c          | 57 +++++++++++++-------------------
 include/grub/serial.h            |  4 +++
 5 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/grub-core/term/arc/serial.c b/grub-core/term/arc/serial.c
index 651f814ee..487aa1b30 100644
--- a/grub-core/term/arc/serial.c
+++ b/grub-core/term/arc/serial.c
@@ -106,6 +106,10 @@ grub_arcserial_add_port (const char *path)
   struct grub_serial_port *port;
   grub_err_t err;
 
+  FOR_SERIAL_PORTS (port)
+    if (grub_strcmp(path, port->name) == 0)
+      return port;
+
   port = grub_zalloc (sizeof (*port));
   if (!port)
     return NULL;
diff --git a/grub-core/term/ieee1275/serial.c b/grub-core/term/ieee1275/serial.c
index b4aa9d5c5..afbe8dcda 100644
--- a/grub-core/term/ieee1275/serial.c
+++ b/grub-core/term/ieee1275/serial.c
@@ -227,6 +227,10 @@ add_port (struct ofserial_hash_ent *ent)
   if (!ent->shortest)
     return NULL;
 
+  FOR_SERIAL_PORTS (port)
+    if (port->ent == ent)
+      return port;
+
   port = grub_zalloc (sizeof (*port));
   if (!port)
     return NULL;
diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index 1eb6a30b6..074bd08ca 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -364,6 +364,14 @@ grub_serial_ns8250_add_port (grub_port_t port, struct 
grub_serial_config *config
         return &com_ports[i];
       }
 
+  FOR_SERIAL_PORTS (p)
+    if (!p->mmio && p->port == port)
+      {
+        if (config != NULL)
+          grub_serial_port_configure (p, config);
+        return p;
+      }
+
   grub_outb (0x5a, port + UART_SR);
   if (grub_inb (port + UART_SR) != 0x5a)
     return NULL;
@@ -409,6 +417,14 @@ grub_serial_ns8250_add_mmio (grub_addr_t addr, unsigned 
int acc_size,
         return &com_ports[i];
       }
 
+  FOR_SERIAL_PORTS (p)
+    if (p->mmio && p->mmio_base == addr)
+      {
+        if (config != NULL)
+          grub_serial_port_configure (p, config);
+        return p;
+      }
+
   p = grub_malloc (sizeof (*p));
   if (p == NULL)
     return NULL;
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index 275dd61af..e214d773b 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -37,8 +37,6 @@
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
-#define FOR_SERIAL_PORTS(var) FOR_LIST_ELEMENTS((var), (grub_serial_ports))
-
 enum
   {
     OPTION_UNIT,
@@ -65,7 +63,7 @@ static const struct grub_arg_option options[] =
   {0, 0, 0, 0, 0, 0}
 };
 
-static struct grub_serial_port *grub_serial_ports;
+struct grub_serial_port *grub_serial_ports;
 
 struct grub_serial_output_state
 {
@@ -147,26 +145,30 @@ grub_serial_find (const char *name)
 {
   struct grub_serial_port *port;
 
+  /*
+   * First look for an exact match by name, this will take care of
+   * things like "com0" which have already been created and that
+   * this function cannot re-create.
+   */
   FOR_SERIAL_PORTS (port)
     if (grub_strcmp (port->name, name) == 0)
-      break;
+      return port;
 
 #if (defined(__mips__) || defined (__i386__) || defined (__x86_64__)) && 
!defined(GRUB_MACHINE_EMU) && !defined(GRUB_MACHINE_ARC)
-  if (!port && grub_strncmp (name, "port", sizeof ("port") - 1) == 0
+  if (grub_strncmp (name, "port", sizeof ("port") - 1) == 0
       && grub_isxdigit (name [sizeof ("port") - 1]))
     {
       port = grub_serial_ns8250_add_port (grub_strtoul (&name[sizeof ("port") 
- 1],
                                                        0, 16), NULL);
-      if (port == NULL)
-        return NULL;
+      if (port != NULL)
+        return port;
     }
-  if (!port && grub_strncmp (name, "mmio,", sizeof ("mmio,") - 1) == 0
+  if (grub_strncmp (name, "mmio,", sizeof ("mmio,") - 1) == 0
       && grub_isxdigit (name [sizeof ("mmio,") - 1]))
     {
       const char *p1, *p = &name[sizeof ("mmio,") - 1];
       grub_addr_t addr = grub_strtoul (p, &p1, 16);
       unsigned int acc_size = 1;
-      unsigned int nlen = p1 - p;
 
       /*
        * If we reach here, we know there's a digit after "mmio,", so
@@ -203,45 +205,32 @@ grub_serial_find (const char *name)
             grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Incorrect MMIO access 
size"));
           }
 
-      /*
-       * Specifying the access size is optional an 
grub_serial_ns8250_add_mmio()
-       * will not add it to the name. So the original loop trying to match an
-       * existing port above might have missed this one. Let's do another
-       * search ignoring the access size part of the string. At this point
-       * nlen contains the length of the name up to the end of the address.
-       */
-      FOR_SERIAL_PORTS (port)
-        if (grub_strncmp (port->name, name, nlen) == 0) {
-          break;
-        }
-
       port = grub_serial_ns8250_add_mmio (addr, acc_size, NULL);
-      if (port == NULL)
-        return NULL;
+      if (port != NULL)
+        return port;
     }
-  if (!port && grub_strcmp (name, "auto") == 0)
+  if (grub_strcmp (name, "auto") == 0)
     {
       /* Look for an SPCR if any. If not, default to com0 */
       port = grub_ns8250_spcr_init ();
-      if (port == NULL)
-        {
-          FOR_SERIAL_PORTS (port)
-            if (grub_strcmp (port->name, "com0") == 0)
-              break;
-       }
+      if (port != NULL)
+        return port;
+      FOR_SERIAL_PORTS (port)
+        if (grub_strcmp (port->name, "com0") == 0)
+          return port;
     }
 #endif
 
 #ifdef GRUB_MACHINE_IEEE1275
-  if (!port && grub_strncmp (name, "ieee1275/", sizeof ("ieee1275/") - 1) == 0)
+  if (grub_strncmp (name, "ieee1275/", sizeof ("ieee1275/") - 1) == 0)
     {
       port = grub_ofserial_add_port (&name[sizeof ("ieee1275/") - 1]);
-      if (port == NULL)
-        return NULL;
+      if (port != NULL)
+        return port;
     }
 #endif
 
-  return port;
+  return NULL;
 }
 
 static grub_err_t
diff --git a/include/grub/serial.h b/include/grub/serial.h
index 17ee0f08b..ce3ec10ec 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -219,4 +219,8 @@ extern void grub_serial_init (void);
 extern void grub_serial_fini (void);
 #endif
 
+extern struct grub_serial_port *grub_serial_ports;
+#define FOR_SERIAL_PORTS(var) FOR_LIST_ELEMENTS((var), (grub_serial_ports))
+
+
 #endif
-- 
2.34.1