Possible bugs & patch...

Richard Mortier Richard.Mortier at cl.cam.ac.uk
Fri Apr 7 17:39:05 EDT 2000


Hi,

Thanks for releasing this.  In trying to build in on RedHat
6.2beta (linux 2.2.14) using (the very pedantic) gcc 2.95.2, I
came across a few problems.  Most of them are just void* implicit
cast problems (no longer allowed in ANSI C++) for return values
from kmalloc(), and a few places where function prototypes are
lacking.  Another was just an irritation: the configure script
assuming linux source was in /usr/src/linux but I'm testing on a
different machine to my build machine, so this isn't the case; I
added an option "--linuxdir" to the configure script to set this
(defaults to /usr/src/linux).

A couple seem a bit more serious/I'm more likely to have got the
fix wrong: 

-- ./elements/linuxmodule/todevice.cc:411 : return is left void
   where I _think_ it should be -1 judging by the test in the
   calling function;

-- ./lib/glue.cc:146 : endptr is "char**" and orig_nptr is "const
   char*", so "*endptr = *orig_nptr" seems dubious;

-- ./linuxmodule/proc_mgr.cc:84 : if the proc entry can't be
   registered in the parent dir., then the created entry and its
   children are recursively destroyed, and the final return value
   from click_kill_dynamic_pde() becomes the return value from
   click_new_dynamic_pde() -- but the former is an int, and the
   latter is a "struct proc_dir_entry*", which also seems
   dubious...

The patch is appended to this mail.

Cheers,

--
Richard Mortier                 --------------------
-----------------        ------ Computer Laboratory,
rmm1002 at cam.ac.uk  ----- University of Cambridge, UK
mort at ieee.org ---- http://www.cl.cam.ac.uk/~rmm1002/

diff -ru click-1.0.2.pristine/configure click-1.0.2/configure
--- click-1.0.2.pristine/configure	Mon Mar 20 11:48:46 2000
+++ click-1.0.2/configure	Fri Apr  7 13:32:33 2000
@@ -59,6 +59,7 @@
 oldincludedir='/usr/include'
 infodir='${prefix}/info'
 mandir='${prefix}/man'
+linuxdir='/usr/src/linux'
 
 # Initialize some other variables.
 subdirs=
@@ -390,6 +391,9 @@
   | --x-librar=* | --x-libra=* | --x-libr=* | --x-lib=* | --x-li=* | --x-l=*)
     x_libraries="$ac_optarg" ;;
 
+  -linuxdir=* | --linuxdir=*)
+    linuxdir="$ac_optarg" ;;
+
   -*) { echo "configure: error: $ac_option: invalid option; use --help to show usage" 1>&2; exit 1; }
     ;;
 
@@ -1580,18 +1584,18 @@
 
 
 
-if test -d /usr/src/linux -a -d /usr/include/linux; then
+if test -d $linuxdir -a -d /usr/include/linux; then
   ac_under_linux=y
 else
   ac_under_linux=n
-  echo "configure: warning: can't find /usr/src/linux or /usr/include/linux: not compiling for Linux" 1>&2
+  echo "configure: warning: can't find $linuxdir or /usr/include/linux: not compiling for Linux" 1>&2
 fi
 
 
 if test $ac_under_linux = y; then
 
-if test ! -f /usr/src/linux/System.map; then
-  { echo "configure: error: can't find /usr/src/linux/System.map" 1>&2; exit 1; }
+if test ! -f $linuxdir/System.map; then
+  { echo "configure: error: can't find $linuxdir/System.map" 1>&2; exit 1; }
 fi
 
 echo $ac_n "checking for Click Linux kernel extensions""... $ac_c" 1>&6
@@ -1599,7 +1603,7 @@
 if eval "test \"`echo '$''{'ac_cv_click_kernel'+set}'`\" = set"; then
   echo $ac_n "(cached) $ac_c" 1>&6
 else
-  if grep register_net_out /usr/src/linux/System.map >/dev/null 2>&1; then
+  if grep register_net_out $linuxdir/System.map >/dev/null 2>&1; then
   ac_cv_click_kernel=yes
 else ac_cv_click_kernel=no; fi
 fi
@@ -1617,7 +1621,7 @@
 if eval "test \"`echo '$''{'ac_cv_read_net_skbcount'+set}'`\" = set"; then
   echo $ac_n "(cached) $ac_c" 1>&6
 else
-  if grep read_net_skbcount /usr/src/linux/System.map >/dev/null 2>&1; then
+  if grep read_net_skbcount $linuxdir/System.map >/dev/null 2>&1; then
   ac_cv_read_net_skbcount=yes
 else ac_cv_read_net_skbcount=no; fi
 fi
@@ -1635,7 +1639,7 @@
 if eval "test \"`echo '$''{'ac_cv_tcp_prot'+set}'`\" = set"; then
   echo $ac_n "(cached) $ac_c" 1>&6
 else
-  if grep tcp_prot /usr/src/linux/net/netsyms.c >/dev/null 2>&1; then
+  if grep tcp_prot $linuxdir/net/netsyms.c >/dev/null 2>&1; then
   ac_cv_tcp_prot=yes
 else ac_cv_tcp_prot=no; fi
 fi
@@ -2176,6 +2180,7 @@
 s%@sharedstatedir@%$sharedstatedir%g
 s%@localstatedir@%$localstatedir%g
 s%@libdir@%$libdir%g
+s%@linuxdir@%$linuxdir%g
 s%@includedir@%$includedir%g
 s%@oldincludedir@%$oldincludedir%g
 s%@infodir@%$infodir%g
diff -ru click-1.0.2.pristine/elements/ip/lookupiproutelinux.cc click-1.0.2/elements/ip/lookupiproutelinux.cc
--- click-1.0.2.pristine/elements/ip/lookupiproutelinux.cc	Sun Jan 23 05:01:41 2000
+++ click-1.0.2/elements/ip/lookupiproutelinux.cc	Fri Apr  7 14:23:32 2000
@@ -23,6 +23,7 @@
 extern "C" {
 # define new xxx_new
 # include <linux/netdevice.h>
+# include <linux/ip.h>
 # include <net/route.h>
 # undef new
 }
diff -ru click-1.0.2.pristine/elements/linuxmodule/fromlinux.cc click-1.0.2/elements/linuxmodule/fromlinux.cc
--- click-1.0.2.pristine/elements/linuxmodule/fromlinux.cc	Sun Jan 23 05:01:41 2000
+++ click-1.0.2/elements/linuxmodule/fromlinux.cc	Fri Apr  7 14:34:59 2000
@@ -55,10 +55,10 @@
 int
 FromLinux::init_dev(void)
 {
-  if (!(_dev = kmalloc(sizeof(struct device), GFP_KERNEL)))
+  if (!(_dev = (struct device*)kmalloc(sizeof(struct device), GFP_KERNEL)))
     return -ENOMEM;
   memset(_dev, 0, sizeof(struct device));
-  if (!(_dev->name = kmalloc(IFNAMSIZ, GFP_KERNEL)))
+  if (!(_dev->name = (char*)kmalloc(IFNAMSIZ, GFP_KERNEL)))
     return -ENOMEM;
   strncpy(_dev->name, _devname.cc(), IFNAMSIZ);
   _dev->init = fl_init;
@@ -77,7 +77,7 @@
        struct sockaddr d;
   } s;
 
-  if (!(_rt = kmalloc(sizeof(struct rtentry), GFP_KERNEL)))
+  if (!(_rt = (struct rtentry*)kmalloc(sizeof(struct rtentry), GFP_KERNEL)))
     return -ENOMEM;
   memset(_rt, 0, sizeof(struct rtentry));
 
@@ -90,6 +90,9 @@
   _rt->rt_dev = _dev->name;
   return 0;
 }
+
+int devinet_ioctl(unsigned int, void *);
+int ip_rt_ioctl(unsigned int, void *);
 
 int
 FromLinux::initialize(ErrorHandler *errh)
diff -ru click-1.0.2.pristine/elements/linuxmodule/perfinfo.cc click-1.0.2/elements/linuxmodule/perfinfo.cc
--- click-1.0.2.pristine/elements/linuxmodule/perfinfo.cc	Fri Feb  4 17:11:43 2000
+++ click-1.0.2/elements/linuxmodule/perfinfo.cc	Fri Apr  7 14:37:15 2000
@@ -19,10 +19,10 @@
 #include "glue.hh"
 #include "asm/msr.h"
 
-static unsigned PerfInfo::_init = 0;
-static unsigned PerfInfo::_metric0 = 0;
-static unsigned PerfInfo::_metric1 = 0;
-static HashMap<String, unsigned> PerfInfo::_metrics;
+unsigned PerfInfo::_init = 0;
+unsigned PerfInfo::_metric0 = 0;
+unsigned PerfInfo::_metric1 = 0;
+HashMap<String, unsigned> PerfInfo::_metrics;
 
 PerfInfo *
 PerfInfo::clone() const
diff -ru click-1.0.2.pristine/elements/linuxmodule/todevice.cc click-1.0.2/elements/linuxmodule/todevice.cc
--- click-1.0.2.pristine/elements/linuxmodule/todevice.cc	Wed Feb 16 15:44:44 2000
+++ click-1.0.2/elements/linuxmodule/todevice.cc	Fri Apr  7 14:47:11 2000
@@ -411,7 +411,7 @@
       printk("ToDevice: too small: len %d tailroom %d\n",
              skb1->len, skb_tailroom(skb1));
       kfree_skb(skb1);
-      return;
+      return -1;
     }
     skb_put(skb1, 60 - skb1->len);
   }
diff -ru click-1.0.2.pristine/elements/linuxmodule/tolinux.cc click-1.0.2/elements/linuxmodule/tolinux.cc
--- click-1.0.2.pristine/elements/linuxmodule/tolinux.cc	Sun Jan 23 05:01:41 2000
+++ click-1.0.2/elements/linuxmodule/tolinux.cc	Fri Apr  7 14:48:29 2000
@@ -31,6 +31,8 @@
   return new ToLinux();
 }
 
+void ptype_dispatch(struct sk_buff *, unsigned short);
+
 void
 ToLinux::push(int port, Packet *p)
 {
diff -ru click-1.0.2.pristine/lib/archive.cc click-1.0.2/lib/archive.cc
--- click-1.0.2.pristine/lib/archive.cc	Sun Jan 23 05:01:35 2000
+++ click-1.0.2/lib/archive.cc	Fri Apr  7 14:18:28 2000
@@ -38,6 +38,8 @@
 58    char ar_fmag[2];		// Always contains ARFMAG == "`\n".
 */
 
+long strtol(const char*, char**, int);
+
 static int
 read_int(const char *data, int max_len,
 	 const char *type, ErrorHandler *errh, int base = 10)
diff -ru click-1.0.2.pristine/lib/confparse.cc click-1.0.2/lib/confparse.cc
--- click-1.0.2.pristine/lib/confparse.cc	Wed Feb 16 17:11:08 2000
+++ click-1.0.2/lib/confparse.cc	Fri Apr  7 14:17:40 2000
@@ -395,6 +395,8 @@
     return take == len;
 }
 
+long strtol(const char *, char **, int);
+
 bool
 cp_integer(String str, int base, int &return_value, String *rest = 0)
 {
diff -ru click-1.0.2.pristine/lib/glue.cc click-1.0.2/lib/glue.cc
--- click-1.0.2.pristine/lib/glue.cc	Sun Jan 23 05:01:35 2000
+++ click-1.0.2/lib/glue.cc	Fri Apr  7 13:54:56 2000
@@ -146,7 +146,7 @@
   if (*nptr == '-' || *nptr == '+')
     nptr++;
   if (!isdigit(*nptr)) {
-    *endptr = *orig_nptr;
+    *endptr = (char*)orig_nptr;
     return INT_MIN;
   }
   unsigned long ul = simple_strtoul(nptr, endptr, base);
diff -ru click-1.0.2.pristine/lib/timedelement.hh click-1.0.2/lib/timedelement.hh
--- click-1.0.2.pristine/lib/timedelement.hh	Sun Jan 23 05:01:35 2000
+++ click-1.0.2/lib/timedelement.hh	Fri Apr  7 14:10:48 2000
@@ -13,7 +13,7 @@
   TimedElement();
   ~TimedElement();
   
-  bool timer_scheduled() const	{ return _timer.scheduled(); }
+  bool timer_scheduled() 	{ return _timer.scheduled(); }
   void timer_schedule_next()	{ _timer.schedule_after_ms(_interval_ms); }
   void timer_schedule_after_ms(int ms) { _timer.schedule_after_ms(ms); }
   void timer_unschedule()	{ _timer.unschedule(); }
diff -ru click-1.0.2.pristine/lib/timer.hh click-1.0.2/lib/timer.hh
--- click-1.0.2.pristine/lib/timer.hh	Fri Mar 17 18:15:41 2000
+++ click-1.0.2/lib/timer.hh	Fri Apr  7 13:57:55 2000
@@ -23,7 +23,7 @@
   Timer(Element *);
   ~Timer()				{ unschedule(); }
   
-  bool scheduled() const		{ return timer_pending(&_t); }
+  bool scheduled() 		{ return timer_pending(&_t); }
   
   void schedule_after_ms(int);
   void unschedule()			{ if (scheduled()) del_timer(&_t); }
diff -ru click-1.0.2.pristine/linuxmodule/Makefile.in click-1.0.2/linuxmodule/Makefile.in
--- click-1.0.2.pristine/linuxmodule/Makefile.in	Thu Mar 16 12:57:41 2000
+++ click-1.0.2/linuxmodule/Makefile.in	Fri Apr  7 14:45:35 2000
@@ -10,6 +10,7 @@
 bindir = @bindir@
 sbindir = @sbindir@
 libdir = @libdir@
+linuxdir = @linuxdir@
 
 VPATH = .:$(top_srcdir)/lib:$(top_srcdir)/$(subdir):$(top_srcdir)/elements/linuxmodule at elements_vpath@
 
@@ -54,8 +55,8 @@
 CFLAGS = @CFLAGS_NDEBUG@
 CXXFLAGS = @CXXFLAGS_NDEBUG@
 
-DEFS = @DEFS@ 
-INCLUDES = -I -I. -I$(top_builddir) -I$(srcdir) -I$(top_srcdir) -I$(top_srcdir)/lib -I/usr/src/linux/include
+DEFS = @DEFS@ -DCPU=686
+INCLUDES = -I -I. -I$(top_builddir) -I$(srcdir) -I$(top_srcdir) -I$(top_srcdir)/lib -I$(linuxdir)/include #-I/usr/src/linux/include
 LDFLAGS = @LDFLAGS@
 LIBS = @LIBS@
 
diff -ru click-1.0.2.pristine/linuxmodule/proc_element.cc click-1.0.2/linuxmodule/proc_element.cc
--- click-1.0.2.pristine/linuxmodule/proc_element.cc	Wed Mar 15 04:29:27 2000
+++ click-1.0.2/linuxmodule/proc_element.cc	Fri Apr  7 14:51:55 2000
@@ -331,7 +331,7 @@
   else
     which = (h - root_handlers);
   click_register_new_dynamic_pde
-    (directory, pattern, h->namelen, h->name, which);
+    (directory, pattern, h->namelen, h->name, (void*)which);
 }
 
 //
@@ -439,7 +439,7 @@
     ndigits++;
   if (ndigits != numbers_ndigits) {
     kfree(numbers);
-    numbers = kmalloc(ndigits * top, GFP_ATOMIC);
+    numbers = (char*)kmalloc(ndigits * top, GFP_ATOMIC);
     if (numbers) {
       for (int i = 1; i < top; i++)
 	sprintf(numbers + (i-1)*ndigits, "%d", i);
@@ -450,7 +450,7 @@
   
   if (!numbers) return;
   
-  element_pdes = kmalloc(sizeof(proc_dir_entry *) * 2 * nelements, GFP_ATOMIC);
+  element_pdes = (struct proc_dir_entry**)kmalloc(sizeof(proc_dir_entry *) * 2 * nelements, GFP_ATOMIC);
   if (!element_pdes) return;
   for (int i = 0; i < 2*nelements; i++)
     element_pdes[i] = 0;
diff -ru click-1.0.2.pristine/linuxmodule/proc_mgr.cc click-1.0.2/linuxmodule/proc_mgr.cc
--- click-1.0.2.pristine/linuxmodule/proc_mgr.cc	Sun Jan 23 05:01:36 2000
+++ click-1.0.2/linuxmodule/proc_mgr.cc	Fri Apr  7 15:09:32 2000
@@ -50,7 +50,7 @@
 click_new_dynamic_pde()
 {
   if (!free_pde) {
-    proc_dir_entry *new_pde = kmalloc(sizeof(proc_dir_entry) * 48, GFP_ATOMIC);
+    proc_dir_entry *new_pde = (proc_dir_entry*)kmalloc(sizeof(proc_dir_entry) * 48, GFP_ATOMIC);
     if (!new_pde) return 0;
     new_pde[0].next = all_pde;
     all_pde = new_pde;
@@ -84,8 +84,12 @@
     pde->name = name;
     pde->data = data;
   }
+  /* XXX RMM this doesn't look very safe... (the retval is used in
+   * ./proc_element.cc; explicit cast is mine for the time being
+   * (should only come into play when linux barfs registering one of
+   * the /proc files */
   if (click_register_pde(parent, pde) < 0)
-    return click_kill_dynamic_pde(pde);
+    return (proc_dir_entry*)click_kill_dynamic_pde(pde);
   else
     return pde;
 }
diff -ru click-1.0.2.pristine/tools/udpgen/Makefile.in click-1.0.2/tools/udpgen/Makefile.in
--- click-1.0.2.pristine/tools/udpgen/Makefile.in	Sat Dec 11 06:59:31 1999
+++ click-1.0.2/tools/udpgen/Makefile.in	Fri Apr  7 15:13:31 2000
@@ -10,6 +10,7 @@
 bindir = @bindir@
 sbindir = @sbindir@
 libdir = @libdir@
+linuxdir = @linuxdir@
 
 VPATH = .:$(top_srcdir)/$(subdir)
 
@@ -40,7 +41,7 @@
 CXXFLAGS = @CXXFLAGS_NDEBUG@ -fno-exceptions -fno-rtti
 
 DEFS = @DEFS@ -D__KERNEL__ -DMODULE
-INCLUDES = -I -I. -I$(top_builddir) -I$(srcdir) -I$(top_srcdir) -I/usr/src/linux/include
+INCLUDES = -I -I. -I$(top_builddir) -I$(srcdir) -I$(top_srcdir) -I$(linuxdir)/include # -I/usr/src/linux/include
 LDFLAGS = @LDFLAGS@
 LIBS = @LIBS@
 



More information about the click mailing list