Re: [AD] Re: binary compatibility check

[Elias Pschernig <elias@xxxxxxxxxx>]

On Tue, 2006-02-07 at 19:07 -0800, Chris wrote:
> On Tuesday 07 February 2006 13:49, Elias Pschernig wrote:
> > It seems, a function declared
> > with AL_INLINE can't have #ifs inside it with my mingw version. I'm not
> > really sure what's going on, anyone has an idea how to fix it?
> 
> Well, as it says:
> include/allegro/system.h:87:1: directives may not be used inside a macro

It's strange it works in linux then.


> Since the body won't be in the macro, you can use directives. A third 
> (modified second) method that doesn't involve changing AL_INLINE would be to 
> just leave the body argument empty, and put the body after the macro:
> 
> AL_INLINE(void, func, (args),)
> {
>   body
> }
> 
> I did this, and it works fine in Linux/GCC 3.4.4.

Yes, but it will not work for msvc:

#define AL_INLINE(type, name, args, code)    __inline _AL_DLL type __cdecl name args code END_OF_INLINE(name)

And include/allegro/internal/alconfig.h also has this:

#ifndef AL_INLINE
   #define AL_INLINE(type, name, args, code)       type name args;
#endif

So it seems, there's a case where we ignore the code argument, which
actually is very strange..

Anyway, this leaves solution 1 and 2. Seeing the msvc #define, I'm not
sure how 2 can work either. So the only way is to duplicate the whole
function definition, which the attached patch does.

Since this code looks so ugly now though, I'm wondering about changing
_install_allegro to get a 4th parameter with the compile-time version,
and then simply have this:

#define allegro_init _install_allegro(SYSTEM_AUTODETECT, errno, atexit, MAKE_VERSION(...))

And let install_allegro again be a normal function, which simply calls
_install_allegro. It would mean, only allegro_init could do the version
check, but that should be enough, and all involved code stays clean.

-- 
Elias Pschernig

Index: include/allegro/system.h
===================================================================
--- include/allegro/system.h	(revision 5704)
+++ include/allegro/system.h	(working copy)
@@ -76,6 +76,9 @@
 AL_FUNC(int, _get_allegro_version, (void));
 AL_FUNC(int, _install_allegro, (int system_id, int *errno_ptr, AL_METHOD(int, atexit_ptr, (AL_METHOD(void, func, (void))))));
 
+
+#if ALLEGRO_SUB_VERSION&1
+/* Enforce strict header x.y.z == lib x.y.z for WIP branches */
 AL_INLINE(int, install_allegro, (int system_id, int *errno_ptr,
    AL_METHOD(int, atexit_ptr, (AL_METHOD(void, func, (void))))),
 {
@@ -84,17 +87,30 @@
    int dll_wip = dll_v & 255;
    int dll_ver = dll_v & ~255;
 
-#if ((ALLEGRO_SUB_VERSION&1) == 0)
+   if (dll_v != MAKE_VERSION(ALLEGRO_VERSION, ALLEGRO_SUB_VERSION, 
+      ALLEGRO_WIP_VERSION)) {
+      uszprintf(allegro_error, ALLEGRO_ERROR_SIZE, get_config_text(
+         "The detected dynamic Allegro library (%d.%d.%d) is "
+         "not compatible with this program (%d.%d.%d)."),
+         dll_ver >> 16, (dll_ver >> 8) & 255, dll_wip,
+         ALLEGRO_VERSION, ALLEGRO_SUB_VERSION, ALLEGRO_WIP_VERSION);
+      return !0;
+   }
+   return r;
+})
+#else
+/* Check that the x.y versions match (eg. 4.2 = 4.2) and that the DLL WIP
+ * version is at least as new as the headers we were compiled with */
+AL_INLINE(int, install_allegro, (int system_id, int *errno_ptr,
+   AL_METHOD(int, atexit_ptr, (AL_METHOD(void, func, (void))))),
+{
+   int r = _install_allegro(system_id, errno_ptr, atexit_ptr);
+   int dll_v = _get_allegro_version(); /* This comes from the DLL. */
+   int dll_wip = dll_v & 255;
+   int dll_ver = dll_v & ~255;
 
-   /* Check that the x.y versions match (eg. 4.2 = 4.2) and that the DLL WIP
-    * version is at least as new as the headers we were compiled with */
    if ((dll_ver != MAKE_VERSION(ALLEGRO_VERSION, ALLEGRO_SUB_VERSION, 0)) ||
        (dll_wip < ALLEGRO_WIP_VERSION)) {
-#else
-   /* Enforce strict header x.y.z == lib x.y.z for WIP branches */
-   if (dll_v != MAKE_VERSION(ALLEGRO_VERSION, ALLEGRO_SUB_VERSION, 
-      ALLEGRO_WIP_VERSION)) {
-#endif
       uszprintf(allegro_error, ALLEGRO_ERROR_SIZE, get_config_text(
          "The detected dynamic Allegro library (%d.%d.%d) is "
          "not compatible with this program (%d.%d.%d)."),
@@ -104,6 +120,7 @@
    }
    return r;
 })
+#endif
 
 #define allegro_init()  install_allegro(SYSTEM_AUTODETECT, &errno, (int (*)(void (*)(void)))atexit)
 AL_FUNC(void, allegro_exit, (void));