[quagga-dev 5269] Re: mes_lookup / LOOKUP robustness

Paul Jakma Paul.Jakma at Sun.COM
Tue Feb 26 22:06:52 GMT 2008


On Tue, 26 Feb 2008, Denis Ovsienko wrote:

> I recall printf substituting NULL with "(nil)", at least in Linux.

Well, GNU libc, apparently.

> Other implementations may tolerate NULL args differently though.

Yep. See also:

  http://www.opensolaris.org/jive/thread.jspa;?messageID=46911

Also, is it my imagination, or is there a problem in that a lot of the 
(struct message) arrays don't have a 0-keyed value in position 0. E.g.:

 	struct message foo_messags [] = {
 		{ FOO_BAR, "foo_bar" },
 		{ FOO_BOB, "Bob" },
 		...
 		{ 0, NULL },
 	};

Seems to me the indexes are off in such cases, so the 'fast-path' in 
mes_lookup wont get hit. This seems typical for a good few of these 
structs. I reckon it's better to handle this all in mes_lookup than 
adding 0s (cause I tend not to remember which enums/defines start at 1 
and which at 0 when reviewing code).

E.g. a revised patch:

Index: lib/log.c
===================================================================
RCS file: /var/cvsroot/quagga/lib/log.c,v
retrieving revision 1.33
diff -u -p -r1.33 log.c
--- lib/log.c	6 Aug 2007 15:21:45 -0000	1.33
+++ lib/log.c	26 Feb 2008 22:06:43 -0000
@@ -756,10 +756,18 @@ lookup (struct message *mes, int key)
  const char *
  mes_lookup (struct message *meslist, int max, int index)
  {
+  const char *str = "(nil)";
+  int shift;
+ 
+  /* key number ranges could start anywhere, e.g. 1 is typical for protocol
+     constants */
+  shift = meslist[0].key;
+
    /* first check for best case: index is in range and matches the key
       value in that slot */
-  if ((index >= 0) && (index < max) && (meslist[index].key == index))
-    return meslist[index].str;
+  if ((index >= shift) && ((index - shift) < max)
+      && (meslist[index - shift].key == index))
+    return meslist[index - shift].str;

    /* fall back to linear search */
    {
@@ -769,14 +777,16 @@ mes_lookup (struct message *meslist, int
        {
  	if (meslist->key == index)
  	  {
+	    str = (meslist->str ? meslist->str : "(nil)");
+
  	    zlog_debug ("message index %d [%s] found in position %d (max is %d)",
-		      index, meslist->str, i, max);
-	    return meslist->str;
+		      index, str, i, max);
+	    return str;
  	  }
        }
    }
    zlog_err("message index %d not found (max is %d)", index, max);
-  return NULL;
+  return str ;
  }

  /* Wrapper around strerror to handle case where it returns NULL. */

regards,
-- 
Paul Jakma,
Solaris Networking                       Sun Microsystems, Scotland
http://opensolaris.org/os/project/quagga tel: EMEA x73150 / +44 15066 73150



More information about the Quagga-dev mailing list