Re: less mixing of object and function pointers

[Ralf Wildenhues]

Hello Peter, Eric,

Here's a patch to fix the issue without changing the public interface:
instead of casting a pointer to void to a function pointer, we pass a
pointer to a function pointer; this requires one more dereference, but
as it's a pointer to data, it eliminates the conversion issue.

And since this functionality has so far never been tested anywhere,
let's add a quick check to mdemo.  The use of my_dirname should be
portable for the purpose of the test issues, and avoids the need for
testing of dirname here.  The move of the test suite to ISO C89 will
be completed in a followup patch.

OK to apply?

Cheers,
Ralf

        Avoid mixing functions and data pointers in callback functions.

        * libltdl/ltdl.c (file_worker_func): New type.
        (lt_dlforeachfile): Instead of passing a function pointer as a
        data pointer, pass a pointer to a file_worker_func pointer.
        (foreach_callback_func): Adjust.
        * tests/mdemo/main.c: Assume C89. Exercise lt_dlforeachfile.
        (callback, try_iterate, my_dirname): New functions.
        * tests/mdemo-exec.test: Check for its output.

Index: libltdl/ltdl.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.241
diff -u -r1.241 ltdl.c
--- libltdl/ltdl.c      30 Aug 2006 04:14:21 -0000      1.241
+++ libltdl/ltdl.c      31 Aug 2006 03:23:33 -0000
@@ -86,6 +86,8 @@
 /* The type of a function used at each iteration of  foreach_dirinpath().  */
 typedef int    foreach_callback_func (char *filename, void *data1,
                                       void *data2);
+/* foreachfile_callback itself calls a function of this type: */
+typedef int    file_worker_func      (const char *filename, void *data);
 
 static int     foreach_dirinpath     (const char *search_path,
                                       const char *base_name,
@@ -1604,8 +1606,7 @@
 static int
 foreachfile_callback (char *dirname, void *data1, void *data2)
 {
-  int (*func) (const char *filename, void *data)
-       = (int (*) (const char *filename, void *data)) data1;
+  file_worker_func *func = *(file_worker_func **) data1;
 
   int    is_done  = 0;
   char   *argz     = 0;
@@ -1642,37 +1643,38 @@
                  void *data)
 {
   int is_done = 0;
+  file_worker_func **fpptr = &func;
 
   if (search_path)
     {
       /* If a specific path was passed, search only the directories
         listed in it.  */
       is_done = foreach_dirinpath (search_path, 0,
-                                  foreachfile_callback, func, data);
+                                  foreachfile_callback, fpptr, data);
     }
   else
     {
       /* Otherwise search the default paths.  */
       is_done = foreach_dirinpath (user_search_path, 0,
-                                  foreachfile_callback, func, data);
+                                  foreachfile_callback, fpptr, data);
       if (!is_done)
        {
          is_done = foreach_dirinpath (getenv(LTDL_SEARCHPATH_VAR), 0,
-                                      foreachfile_callback, func, data);
+                                      foreachfile_callback, fpptr, data);
        }
 
 #if defined(LT_MODULE_PATH_VAR)
       if (!is_done)
        {
          is_done = foreach_dirinpath (getenv(LT_MODULE_PATH_VAR), 0,
-                                      foreachfile_callback, func, data);
+                                      foreachfile_callback, fpptr, data);
        }
 #endif
 #if defined(LT_DLSEARCH_PATH)
       if (!is_done && sys_dlsearch_path)
        {
          is_done = foreach_dirinpath (sys_dlsearch_path, 0,
-                                      foreachfile_callback, func, data);
+                                      foreachfile_callback, fpptr, data);
        }
 #endif
     }
Index: tests/mdemo-exec.test
===================================================================
RCS file: /cvsroot/libtool/libtool/tests/mdemo-exec.test,v
retrieving revision 1.19
diff -u -r1.19 mdemo-exec.test
--- tests/mdemo-exec.test       22 Apr 2005 10:10:30 -0000      1.19
+++ tests/mdemo-exec.test       31 Aug 2006 03:23:33 -0000
@@ -27,7 +27,9 @@
 
 func_rmprefixdir
 func_exec_init "uninstalled"
-func_exec "tests/mdemo/mdemo_static tests/mdemo/foo1.la tests/mdemo/libfoo2.la"
-func_exec "tests/mdemo/mdemo tests/mdemo/foo1.la tests/mdemo/libfoo2.la"
+func_exec "tests/mdemo/mdemo_static tests/mdemo/foo1.la 
tests/mdemo/libfoo2.la" \
+         "try_iterate: .*libfoo2"
+func_exec "tests/mdemo/mdemo tests/mdemo/foo1.la tests/mdemo/libfoo2.la" \
+         "try_iterate: .*libfoo2"
 
 exit $exec_status
Index: tests/mdemo/main.c
===================================================================
RCS file: /cvsroot/libtool/libtool/tests/mdemo/main.c,v
retrieving revision 1.2
diff -u -r1.2 main.c
--- tests/mdemo/main.c  22 Apr 2005 10:10:31 -0000      1.2
+++ tests/mdemo/main.c  31 Aug 2006 03:23:33 -0000
@@ -1,5 +1,5 @@
 /* main.c -- mdemo test program
-   Copyright (C) 1998-2000 Free Software Foundation, Inc.
+   Copyright (C) 1998-2000, 2006 Free Software Foundation, Inc.
    Originally by Thomas Tanner <address@hidden>
    This file is part of GNU Libtool.
 
@@ -21,10 +21,11 @@
 #include "foo.h"
 #include "ltdl.h"
 #include <stdio.h>
+#include <string.h>
 
 int
 test_dl (filename)
-  char *filename;
+test_dl (char *filename)
 {
   lt_dlhandle handle;  
   const lt_dlinfo *info;
@@ -132,7 +133,7 @@
     return 1;
   }
 
-  pmyfunc = (int(*)())lt_dlsym(handle, "myfunc");  
+  pmyfunc = (int(*)())lt_dlsym(handle, "myfunc");
   if (pmyfunc)
     {
       int value = (*pmyfunc) ();
@@ -163,10 +164,38 @@
   return ret;
 }
 
+static int
+callback (const char *filename, void *data)
+{
+  printf ("%s: %s\n", (char *)data, filename);
+  return 0;
+}
+
+static int
+try_iterate (const char *search_path)
+{
+  char *s = "try_iterate";
+  return lt_dlforeachfile (search_path, callback, s);
+}
+
+/* cheap dirname clone.  We require a '/' separator, nonempty and large
+   enough input, not ending with '/', and we will overwrite the input. */
+static char *
+my_dirname (char *path)
+{
+  char *p = strrchr (path, '/');
+  if (p)
+    *p = '\0';
+  else
+    {
+      path[0] = '.';
+      path[1] = '\0';
+    }
+  return path;
+}
+
 int
-main (argc, argv)
-  int argc;
-  char **argv;
+main (int argc, char **argv)
 {
   int i;
   int ret = 0;
@@ -190,6 +219,14 @@
   if (test_dlself())
     ret = 1;
 
+  for (i = 1; i < argc; i++)
+    if (argv[i][0] != '\0')
+      {
+       my_dirname (argv[i]);
+       if (try_iterate (argv[i]))
+         ret = 1;
+      }
+
   lt_dlexit();
   return ret;
 }