[quagga-dev 3647] Re: 0.98.4 bug report: files ospf_opaque.c, ospf_vty.c and ospf6_asbr.c

Andrew J. Schorr aschorr at telemetry-investments.com
Thu Sep 15 14:47:06 BST 2005


Sorry to be so slow to follow up on this.

On Tue, Jul 26, 2005 at 11:07:08AM +0000, d binderman wrote:
> The compiler said
> 
> 1.
> 
> ospf_opaque.c(1993): remark #592: variable "ospf" is used before its value 
> is set
> ospf_opaque.c(2072): remark #592: variable "ospf" is used before its value 
> is set

Are you referring to this code?

   void
   ospf_opaque_lsa_refresh_schedule (struct ospf_lsa *lsa0)
   {
     struct ospf *ospf = ospf;

and

   void
   ospf_opaque_lsa_flush_schedule (struct ospf_lsa *lsa0)
   {
     struct ospf *ospf = ospf;

I must admit that I cannot see any valid reason for doing
that.  I also do not understand why gcc does not issue
a warning.

> ospf_vty.c(6031): remark #592: variable "nexthop" is used before its value 
> is set

Is this referring to this code segment?

   DEFUN (no_ospf_default_information_originate,
	  no_ospf_default_information_originate_cmd,
	  "no default-information originate",
	  NO_STR
	  "Control distribution of default information\n"
	  "Distribute a default route\n")
   {
     struct ospf *ospf = vty->index;
     struct prefix_ipv4 p;
     struct in_addr nexthop;

     p.family = AF_INET;
     p.prefix.s_addr = 0;
     p.prefixlen = 0;

     ospf_external_lsa_flush (ospf, DEFAULT_ROUTE, &p, 0, nexthop);

If so, this looks like a bug.  On the other hand, the
ospf_external_lsa_flush function does not seem to use the value
of the final argument (the only code that referenced it is
commented out).  I'm not sure what the proper fix is
here.  Given the existing code, it might make sense to drop
the final argument to the function...

> Suggest initialise local variables before first use.
> 
> 2.
> 
> ospf6_asbr.c(456): warning #175: subscript out of range
> ospf6_asbr.c(759): warning #175: subscript out of range
> ospf6_asbr.c(762): warning #175: subscript out of range
> ospf6_asbr.c(798): warning #175: subscript out of range
> ospf6_asbr.c(803): warning #175: subscript out of range
> ospf6_asbr.c(1208): warning #175: subscript out of range
> 
> There is a bug in the ZROUTE_NAME macro.

Perhaps I'm being dense, but I'm not seeing this.

In lib/zebra.h:

   #define ZEBRA_ROUTE_MAX                  11

In ospf6d/ospf6_asbr.c:

   const char *zroute_name[] =
   { "system", "kernel", "connected", "static",
     "rip", "ripng", "ospf", "ospf6", "isis", "bgp", "hsls", "unknown" };

   #define ZROUTE_NAME(x)                                     \
     (0 < (x) && (x) < ZEBRA_ROUTE_MAX ? zroute_name[(x)] :   \
      zroute_name[ZEBRA_ROUTE_MAX])

Note that zroute_name[] has 12 entries (subscripts 0 through 11).
So it looks like the subscript is in range to me, although the first
test for '0 < (x)' seems incorrect; shouldn't it be '0 <= (x)'?
But that still doesn't cause a subscript out of range.

Am I missing something?

Possible patch attached.  But there don't seem to be any real
bugs here...

Regards,
Andy
-------------- next part --------------
Index: ospfd/ospf_opaque.c
===================================================================
RCS file: /var/cvsroot/quagga/ospfd/ospf_opaque.c,v
retrieving revision 1.16
diff -b -u -p -r1.16 ospf_opaque.c
--- ospfd/ospf_opaque.c	11 May 2005 18:09:59 -0000	1.16
+++ ospfd/ospf_opaque.c	15 Sep 2005 13:45:44 -0000
@@ -1982,7 +1982,7 @@ out:
 void
 ospf_opaque_lsa_refresh_schedule (struct ospf_lsa *lsa0)
 {
-  struct ospf *ospf = ospf;
+  struct ospf *ospf;
   struct opaque_info_per_type *oipt;
   struct opaque_info_per_id *oipi;
   struct ospf_lsa *lsa;
@@ -2061,7 +2061,7 @@ ospf_opaque_lsa_refresh_timer (struct th
 void
 ospf_opaque_lsa_flush_schedule (struct ospf_lsa *lsa0)
 {
-  struct ospf *ospf = ospf;
+  struct ospf *ospf;
   struct opaque_info_per_type *oipt;
   struct opaque_info_per_id *oipi;
   struct ospf_lsa *lsa;
Index: ospf6d/ospf6_asbr.c
===================================================================
RCS file: /var/cvsroot/quagga/ospf6d/ospf6_asbr.c,v
retrieving revision 1.18
diff -b -u -p -r1.18 ospf6_asbr.c
--- ospf6d/ospf6_asbr.c	10 Aug 2005 15:46:11 -0000	1.18
+++ ospf6d/ospf6_asbr.c	15 Sep 2005 13:45:44 -0000
@@ -50,18 +50,19 @@
 
 unsigned char conf_debug_ospf6_asbr = 0;
 
-const char *zroute_name[] =
+/* Note: these descriptions match the route types defined in lib/zebra.h */
+const char *zroute_name[ZEBRA_ROUTE_MAX+1] =
 { "system", "kernel", "connected", "static",
   "rip", "ripng", "ospf", "ospf6", "isis", "bgp", "hsls", "unknown" };
 
-const char *zroute_abname[] =
+const char *zroute_abname[ZEBRA_ROUTE_MAX+1] =
 { "X", "K", "C", "S", "R", "R", "O", "O", "I", "B", "H", "?" };
 
 #define ZROUTE_NAME(x)                                     \
-  (0 < (x) && (x) < ZEBRA_ROUTE_MAX ? zroute_name[(x)] :   \
+  (0 <= (x) && (x) < ZEBRA_ROUTE_MAX ? zroute_name[(x)] :   \
    zroute_name[ZEBRA_ROUTE_MAX])
 #define ZROUTE_ABNAME(x)                                   \
-  (0 < (x) && (x) < ZEBRA_ROUTE_MAX ? zroute_abname[(x)] : \
+  (0 <= (x) && (x) < ZEBRA_ROUTE_MAX ? zroute_abname[(x)] : \
    zroute_abname[ZEBRA_ROUTE_MAX])
 
 /* AS External LSA origination */


More information about the Quagga-dev mailing list