Re: LibIDN code quality suggestions

[Bittner Ede]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello Simon,

        Thank you for your fast reply!
        It's nice to have the fix for first problem. The second is big
dilemma. As I see in our projects, as they grow bigger and bigger, we
faced many problems around the integer values. In this case the
easiest way - and the compatible way - is to convert everything to
integer (and #define the values). At long term the enum is the better
way. The consistency that the compiler provides for enums is great,
but I think you know that.
        I think the soultion is that idna_rc should be changed to int in the
next minor release for all *_strerror, but in the future at as code
reaches the next major version, the API can be rewritten, to use enums
everywhere. As the result everyone can use the old version without
code change, or the new next major version, with some minimal code
rewrite.

On 2011.10.19. 11:18, Simon Josefsson wrote:
> Bittner Ede <address@hidden> writes:
> 
>> Dear Development team,
>> 
>> I've tasked at my workplace, to use libidn. I've found some
>> minor issues with the library 1, the idn-free function (in
>> idn-free.h) misses the EXTERN "C" -qualifier, so when I included
>> in the C++, I've get a link error. My workaround was to include
>> the file with extern "C", but I think it isnt the preferable way,
>> because all other header in the library do not need this. I've
>> understand the comaplaints about the idn_free, but our production
>> enviroment requires cross platform compatible code, so normal
>> free is not an option for us.
> 
> Hello Bittner.  Thank you for your attention to detail and your
> report.
> 
> It was a mistake, idn-free.h should also have the extern "C"
> marker.  I have fixed this, it will be part of the next release.
> 
>> 2, all of the idna_* function return int value, but the
>> idna_strerror takes the Idna_rc enum type as it's input
>> parameter, so I've to static_cast the return value. This can
>> easily avoided if the function returns Idna_rc instead of int.
>> (And it will improve code quality as well)
> 
> This will require changing almost all APIs in the library...  I'm a
> bit hesitant to do that -- it seems to me as if that would trigger
> a lot of warnings in existing Libidn applications that use 'int'
> for the return codes and now have to change to use 'Idna_rc' to
> avoid a similar warning as you noticed.  Right?
> 
> How about if we make idna_strerror (and the other *_strerror
> functions) take an int argument instead?  Then at least all library
> functions are type-equal when it comes to error codes.
> 
> Cheers, /Simon
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6em8oACgkQRg1a/GImAJIwZgCgmI5MCldLtQPQzDt8aJ5Zy7As
HisAnREtKwZctDV9K761p+E2Cht+LW97
=TQjq
-----END PGP SIGNATURE-----