Re: [AD] version check in al_init

[Peter Wang <novalazy@xxxxxxxxxx>]

On 2010-03-01, Elias Pschernig <elias.pschernig@xxxxxxxxxx> wrote:
> Just spent some time wondering about weird looking graphics and strange
> crashes until I figured out I had compiled my code with the SVN headers
> but linked against the 4.9.17 libraries.
> 
> The attached patch prevents that, like we do it in A4. It still doesn't
> prevent you from loading incompatible addons - not sure how we could
> solve that as no macros are involved... maybe add some wrapper macros
> for the addon init functions.

I don't think it's that important.

> diff --git a/include/allegro5/system_new.h b/include/allegro5/system_new.h
> index 058cfa9..5191ae3 100644
> --- a/include/allegro5/system_new.h
> +++ b/include/allegro5/system_new.h
> @@ -12,10 +12,12 @@ typedef struct ALLEGRO_SYSTEM ALLEGRO_SYSTEM;
>  
>  /* Function: al_init
>   */
> -#define al_init()    (al_install_system(atexit))
> +#define al_init()    (_al_install_system_version_check(atexit, ALLEGRO_VERSION_INT))
>  
>  AL_FUNC(bool, al_install_system, (int (*atexit_ptr)(void (*)(void))));
>  AL_FUNC(void, al_uninstall_system, (void));
> +AL_FUNC(bool, _al_install_system_version_check,
> +   (int (*atexit_ptr)(void (*)(void)), uint32_t header_version));
>  AL_FUNC(ALLEGRO_SYSTEM *, al_get_system_driver, (void));
>  AL_FUNC(ALLEGRO_CONFIG *, al_get_system_config, (void));

Hmm, so we avoid the check if al_install_system is called directly.

> diff --git a/src/system_new.c b/src/system_new.c
> index aa81d6c..b04bbe6 100644
> --- a/src/system_new.c
> +++ b/src/system_new.c
> @@ -226,6 +226,41 @@ bool al_install_system(int (*atexit_ptr)(void (*)(void)))
>  
>  
>  
> +bool compatible_versions(int a, int b)

static

> +{
> +   /* An x.y.*.* version is never compatible with anything but
> +    * another x.y.*.* version.
> +    */
> +   if ((a & 0xffff0000) != (b & 0xffff0000)) {
> +      return false;
> +   }
> +   /* An x.y.z.* version is only compatible with x.y.z.* if y is odd.
> +    * If y is event then all x.y.*.* are compatible.
> +    */

even

I didn't find the comments that clear. Maybe something like:

    Let a = xa.ya.za.*
    Let b = xb.yb.zb.*
    When y is odd, a is compatible with b if xa.ya.za = xb.yb.zb.
    When y is even, a is compatible with b if xa.ya = xb.yb.
    Otherwise a and b are incompatible.

> +   if (((a & 0x00ff0000) >> 16) & 1) {
> +      
> +      if ((a & 0x0000ff00) != (b & 0x0000ff00)) {
> +         return false;
> +      }
> +   }
> +   return true;
> +}
> +
> +
> +
> +bool _al_install_system_version_check(int (*atexit_ptr)(void (*)(void)),
> +   uint32_t header_version)
> +{
> +   uint32_t library_version = al_get_allegro_version();
> +   /* Note: We cannot call logging functions yet.
> +    * TODO: Maybe we want to do the check after the "bootstrap" system
> +    * is available at least?
> +    */
> +   if (!compatible_versions(header_version, library_version)) return false;
> +   return al_install_system(atexit_ptr);
> +}

(When you write if statements like that, it takes me an extra second to read.)

Peter