Re: [AD] version check in al_init

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]




On Tue, Mar 2, 2010 at 12:51 AM, Peter Wang <novalazy@xxxxxxxxxxxxxxxxx> wrote:
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.

It's hard to say. Everyone using the 4.9.x branch can be affected, and once we are on 5.1.x many will have the incompatible 5.0.x libraries around. The patch doesn't actually fix my problem since 4.9.17 doesn't have the check, so I'm thinking maybe the check is better put into the macro itself? Something like:

#define al_init() al_is_version_ok(ALLEGRO_VERSION) ? al_install_system() : false


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


Yeah. Guess we could say like the addons, it's not so important. Doing it for only al_init of course means users will hit the problem regardless. It's also easy enough to do the check yourself, in fact when we added all the al_get_*_version() functions I wanted to add the checks to my wrapper to prevent this problem :)
 
> 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.


Yeah, I was a bit too tired yesterday to make a patch, half of the comments say the opposite of what was intended :P

> +   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.)


Yeah, not coding in C/C++ much these days so I tend to forget about the style guidelines.


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/