Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
Gary V. Vaughan gary at gnu.org writes: Does this work with C and C++ compilers? Yes. I took that as a yes vote and pushed the patch. :) Well, almost. The MALLOC macro already includes a cast, as part of allocating an array of n elements of the type. And lt_dlhandle-deplibs is an array of lt_dlhandle (ie. lt_dlhandle*, ie. lt__handle**). Therefore, the original code was a) mallocing the wrong size [n*sizeof (struct lt__handle) instead of n*sizeof (lt_dlhandle)], b) explicitly casting to the wrong type [lt_dlhandle instead of lt_dlhandle*] in spite of the implicit cast given by MALLOC. So I'm committing this as obvious. From fe9c6440a352de1d7b2d148fd4393c669927ac82 Mon Sep 17 00:00:00 2001 From: Eric Blake [EMAIL PROTECTED] Date: Thu, 24 Apr 2008 10:05:12 -0600 Subject: [PATCH] Fix bug in 2008-04-21 type improvements. * libltdl/ltdl.c (load_deplibs): Fix casting bug. Signed-off-by: Eric Blake [EMAIL PROTECTED] --- ChangeLog |5 + libltdl/ltdl.c |4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9e97b71..737c5c8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2008-04-24 Eric Blake [EMAIL PROTECTED] + + Fix bug in 2008-04-21 type improvements. + * libltdl/ltdl.c (load_deplibs): Fix casting bug. + 2008-04-23 Michael Haubenwallner [EMAIL PROTECTED] Use /nonexistent as destination for files not to be installed. diff --git a/libltdl/ltdl.c b/libltdl/ltdl.c index c8edcb4..a89c6bb 100644 --- a/libltdl/ltdl.c +++ b/libltdl/ltdl.c @@ -921,7 +921,7 @@ load_deplibs (lt_dlhandle handle, char *deplibs) lt_dlhandle cur = handle; int j = 0; - cur-deplibs = (lt_dlhandle) MALLOC (struct lt__handle, depcount); + cur-deplibs = MALLOC (lt_dlhandle, depcount); if (!cur-deplibs) goto cleanup_names; -- 1.5.5.1
Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
Hallo Ralf, On 21 Apr 2008, at 01:23, Ralf Wildenhues wrote: * Gary V. Vaughan wrote on Mon, Apr 21, 2008 at 02:39:37AM CEST: * libltdl/lt__private.h (lt__handle, lt__advise): Typedefs removed in favour of... * libltdl/ltdl.h (lt_dlhandle), libltdl/libltdl/lt_dlloader.h (lt_dladvise): ...changing from void* to forward declarations. * libltdl/loaders/dlopen.c, libltdl/loaders/loadlibrary.c, libltdl/lt_dlloader.c, libltdl/ltdl.c: Remove many redundant type casts. * NEWS: Updated. Suggested by Bob Friesenhahn [EMAIL PROTECTED] Does this work with C and C++ compilers? Yes. Does it change the ABI or API? API, definitely not. ABI, not as far as I understand it. If yes, no, then I like it. Otherwise, as nice as it is, it would be a bit inappropriate for a minor release number, no? I took that as a yes vote and pushed the patch. :) Cheers, Gary -- ())_. Email me: [EMAIL PROTECTED] ( '/ Read my blog: http://blog.azazil.net / )= ...and my book: http://sources.redhat.com/autobook `(_~)_ PGP.sig Description: This is a digitally signed message part
Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
On Mon, 21 Apr 2008, Ralf Wildenhues wrote: Does this work with C and C++ compilers? Does it change the ABI or API? If yes, no, then I like it. Otherwise, as nice as it is, it would be a bit inappropriate for a minor release number, no? I use this approach in GraphicsMagick C code without any problems. If it changes the ABI then the existing code must be broken. It could only be seen as an API change if the using application did not use libtool's specified API types (as it should). We would (of course) test with many compilers prior to release. Bob == Bob Friesenhahn [EMAIL PROTECTED], http://www.simplesystems.org/users/bfriesen/ GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
[PATCH 363] Allow better compile time type checking for lt_dladvise
* libltdl/lt__private.h (lt__handle, lt__advise): Typedefs
removed in favour of...
* libltdl/ltdl.h (lt_dlhandle),
libltdl/libltdl/lt_dlloader.h (lt_dladvise): ...changing from
void* to forward declarations.
* libltdl/loaders/dlopen.c, libltdl/loaders/loadlibrary.c,
libltdl/lt_dlloader.c, libltdl/ltdl.c: Remove many redundant
type casts.
* NEWS: Updated.
Suggested by Bob Friesenhahn [EMAIL PROTECTED]
---
Now we get compile time errors for mistakes like this:
] cat advise.c EOF
#include ltdl.h
lt_dlhandle
my_dlopenext (const char *filename)
{
lt_dlhandle handle;
lt_dladvise advise;
if (lt_dladvise_init (advise) || lt_dladvise_ext (advise))
exit(1);
handle = lt_dlopenadvise (filename, advise);
lt_dladvise_destroy (advise);
return handle;
}
EOF
] gcc -c -I ../../libltdl -o advise.o advise.c
advise.c: In function 'my_dlopenext':
advise.c:11: warning: passing argument 2 of 'lt_dlopenadvise' from incompatible pointer type
Okay to apply?
ChangeLog | 12
NEWS |4 ++
libltdl/libltdl/lt__private.h |8 +--
libltdl/libltdl/lt_dlloader.h |4 +-
libltdl/loaders/dlopen.c |8 ++--
libltdl/loaders/loadlibrary.c |4 +-
libltdl/lt_dlloader.c |4 +-
libltdl/ltdl.c| 113 -
libltdl/ltdl.h|4 +-
9 files changed, 85 insertions(+), 76 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 8d2e5c3..18a58e9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
2008-04-20 Gary V. Vaughan [EMAIL PROTECTED]
+ Allow better compile time type checking for lt_dladvise.
+ * libltdl/lt__private.h (lt__handle, lt__advise): Typedefs
+ removed in favour of...
+ * libltdl/ltdl.h (lt_dlhandle),
+ libltdl/libltdl/lt_dlloader.h (lt_dladvise): ...changing from
+ void* to forward declarations.
+ * libltdl/loaders/dlopen.c, libltdl/loaders/loadlibrary.c,
+ libltdl/lt_dlloader.c, libltdl/ltdl.c: Remove many redundant
+ type casts.
+ * NEWS: Updated.
+ Suggested by Bob Friesenhahn [EMAIL PROTECTED]
+
Fix misleading lt_dlopenadvise documentation.
* doc/libtool.texi (lt_dlopenadvise): The last parameter is of
type lt_dladvise, not lt_dladvise*.
diff --git a/NEWS b/NEWS
index 300a57f..2fd7f0d 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,10 @@ New in 2.2.4: 2008-??-??: CVS version 2.2.3a, Libtool team:
- The documentation for lt_dlopenadvise showed the wrong type for
the lt_dladvise parameter.
+ - The public declarations for lt_dlhandle and lt_dladvise are now
+incomplete struct types rather than void*, which means that nearly
+all casting is eliminated allowing the compiler to provide more
+type checking.
New in 2.2.2: 2008-04-01: CVS version 2.2.1a, Libtool team:
diff --git a/libltdl/libltdl/lt__private.h b/libltdl/libltdl/lt__private.h
index c258df8..4ce936d 100644
--- a/libltdl/libltdl/lt__private.h
+++ b/libltdl/libltdl/lt__private.h
@@ -1,6 +1,6 @@
/* lt__private.h -- internal apis for libltdl
- Copyright (C) 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+ Copyright (C) 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
Written by Gary V. Vaughan, 2004
NOTE: The canonical source of this file is maintained with the
@@ -107,10 +107,8 @@ typedef struct {
void * data;
} lt_interface_data;
-typedef struct lt__handle lt__handle;
-
struct lt__handle {
- lt__handle * next;
+ lt_dlhandle next;
const lt_dlvtable * vtable; /* dlopening interface */
lt_dlinfo info; /* user visible fields */
int depcount; /* number of dependencies */
@@ -121,8 +119,6 @@ struct lt__handle {
int flags; /* various boolean stats */
};
-typedef struct lt__advise lt__advise;
-
struct lt__advise {
unsigned int try_ext:1; /* try system library extensions. */
unsigned int is_resident:1; /* module can't be unloaded. */
diff --git a/libltdl/libltdl/lt_dlloader.h b/libltdl/libltdl/lt_dlloader.h
index 9512b35..ae131fa 100644
--- a/libltdl/libltdl/lt_dlloader.h
+++ b/libltdl/libltdl/lt_dlloader.h
@@ -1,6 +1,6 @@
/* lt_dlloader.h -- dynamic library loader interface
- Copyright (C) 2004, 2007 Free Software Foundation, Inc.
+ Copyright (C) 2004, 2007, 2008 Free Software Foundation, Inc.
Written by Gary V. Vaughan, 2004
NOTE: The canonical source of this file is maintained with the
@@ -38,7 +38,7 @@ LT_BEGIN_C_DECLS
typedef void * lt_dlloader;
typedef void * lt_module;
typedef void * lt_user_data;
-typedef void * lt_dladvise;
+typedef struct lt__advise * lt_dladvise;
/* Function pointer types for module loader vtable entries: */
typedef lt_module lt_module_open (lt_user_data data,
diff --git a/libltdl/loaders/dlopen.c b/libltdl/loaders/dlopen.c
index 94680c5..b445407 100644
--- a/libltdl/loaders/dlopen.c
+++ b/libltdl/loaders/dlopen.c
@@ -174,20 +174,20 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char *filename,
#ifdef
Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
On Sun, 20 Apr 2008, Gary V. Vaughan wrote: * libltdl/lt__private.h (lt__handle, lt__advise): Typedefs removed in favour of... * libltdl/ltdl.h (lt_dlhandle), libltdl/libltdl/lt_dlloader.h (lt_dladvise): ...changing from void* to forward declarations. * libltdl/loaders/dlopen.c, libltdl/loaders/loadlibrary.c, libltdl/lt_dlloader.c, libltdl/ltdl.c: Remove many redundant type casts. * NEWS: Updated. Suggested by Bob Friesenhahn [EMAIL PROTECTED] I like this very much. Besides helping user code, it should diminish the number of compiler warnings when compiling libltdl and may even avoid some optimization-related bugs. Bob == Bob Friesenhahn [EMAIL PROTECTED], http://www.simplesystems.org/users/bfriesen/ GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
Howdy Bob! On 20 Apr 2008, at 21:13, Bob Friesenhahn wrote: On Sun, 20 Apr 2008, Gary V. Vaughan wrote: * libltdl/lt__private.h (lt__handle, lt__advise): Typedefs removed in favour of... * libltdl/ltdl.h (lt_dlhandle), libltdl/libltdl/lt_dlloader.h (lt_dladvise): ...changing from void* to forward declarations. * libltdl/loaders/dlopen.c, libltdl/loaders/loadlibrary.c, libltdl/lt_dlloader.c, libltdl/ltdl.c: Remove many redundant type casts. * NEWS: Updated. Suggested by Bob Friesenhahn [EMAIL PROTECTED] I like this very much. Besides helping user code, it should diminish the number of compiler warnings when compiling libltdl and may even avoid some optimization-related bugs. Is that a review with an implicit please apply? Or are you just cheering me on from the sidelines :-) Cheers, Gary -- ())_. Email me: [EMAIL PROTECTED] ( '/ Read my blog: http://blog.azazil.net / )= ...and my book: http://sources.redhat.com/autobook `(_~)_ PGP.sig Description: This is a digitally signed message part
Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
On Sun, 20 Apr 2008, Gary V. Vaughan wrote: I like this very much. Besides helping user code, it should diminish the number of compiler warnings when compiling libltdl and may even avoid some optimization-related bugs. Is that a review with an implicit please apply? Or are you just cheering me on from the sidelines :-) I reviewed the code updates and liked them very much. Probably I was communicating in American when I should have been communicating in British. Here are my comments translated to British: please apply. :-) Bob == Bob Friesenhahn [EMAIL PROTECTED], http://www.simplesystems.org/users/bfriesen/ GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
Re: [PATCH 363] Allow better compile time type checking for lt_dladvise
* Gary V. Vaughan wrote on Mon, Apr 21, 2008 at 02:39:37AM CEST: * libltdl/lt__private.h (lt__handle, lt__advise): Typedefs removed in favour of... * libltdl/ltdl.h (lt_dlhandle), libltdl/libltdl/lt_dlloader.h (lt_dladvise): ...changing from void* to forward declarations. * libltdl/loaders/dlopen.c, libltdl/loaders/loadlibrary.c, libltdl/lt_dlloader.c, libltdl/ltdl.c: Remove many redundant type casts. * NEWS: Updated. Suggested by Bob Friesenhahn [EMAIL PROTECTED] Does this work with C and C++ compilers? Does it change the ABI or API? If yes, no, then I like it. Otherwise, as nice as it is, it would be a bit inappropriate for a minor release number, no? Cheers, Ralf
