[gnutls-dev] size_t, int and 64 bit machines

[Simon Josefsson]

Peter Valchev <pvalchev at openbsd.org> writes:

> Simon Josefsson <simon <at> josefsson.org> writes:
>> Deanna Phillips <deanna <at> sdf.lonestar.org> writes:
>> > gnutls_x509.c:1897: warning: passing arg 2 of `read_binary_file' from
> incompatible pointer type
>> >
>> > The simple fix for that particular test was -
>> >
>> >    typedef struct
>> >    {
>> >      unsigned char *data;
>> > -    unsigned int size;
>> > +    size_t size;
>> >    } gnutls_datum_t;
>> 
>> I am aware of this one, but the problem is: it is part of gnutls.h and
>> thus part of the external API.  This is really problematic to change,
>> since it won't be backwards compatible.
>> 
>> However, can we get away by considering that GnuTLS doesn't work at
>> all on platforms with 8-byte size_t and just make the change?  We have
>> already some supported 64-bit platforms, e.g. amd64 and ia64, but do
>> they have 8-byte size_t?  I doubt this is a reasonable way forward...
>> 
>> Another alternative is to create a new type 'gnutls_buffer_t' with the
>> proper types, and create new APIs for all existing APIs that use the
>> type.  Which is rather difficult.
>> 
>> A further alternative is to combine the last solution with library
>> versioning, to avoid renaming functions, although this is not very
>> portable as far as I know (?).
>  
> I'm afraid this is a real problem you need to solve.  Mixing int vs long
> vs size_t is bad news because they have different sizes on 64-bit
> architectures (including on linux), so you *need to be consistent*.
> Think of all the problems that can arouse when you mix 2 types of
> different sizes.  Either use int everywhere (do not mix in long and
> size_t, replace those with the proper types instead) or look at Deanna's
> diff which uses size_t consistently throughout and changes the API.

Yup, I know.  The problem isn't realizing that it has to be fixed, but
how to fix it in a good way...

> However you don't *need* to change the API if you fix all usages
> to match it and be consistent (from a very quick look, most of your
> interfaces already use size_t, so...)

...right, and that is the solution I have taken so far.  It is simply
not worth breaking the API to change 'int' to 'size_t' in some places
(e.g., 'gnutls_datum_t').

> Again, as it is, your API uses unsigned int for that function, but
> you have used size_t in places - BAD!

Yup, I installed patches for some of those.  If there are more of
them, please send updated patches!

> If you change the API, you just bump the shared library's major number
> (for example from libgnutls.so.15.20 -> libgnutls.so.16.20).  Old
> applications continue to work (your backwards compatibility concern does
> not exist), and new programs link with the new library, I'm not sure
> what you mean about this "not being portable", that's what shared
> libraries are for.
>
> Recall the simple shared library rules:
> When you change the interface -> bump major
> When you add an interface (eg a new function) -> bump major

The problem is source-level backwards compatibility and existing code
out there that use GnuTLS or libtasn1.  They may have code like, e.g.:

 int len;
...
 asn1_der_coding (element, name, ider, &len, NULL);

If I would have installed Deanna's entire patch, that public API
function of libtasn1 would have changed it's prototype from 'int len'
to 'size_t len'.  Now if 'int' is 4 bytes and 'size_t' is 8 bytes, you
can imagine the problems caused....

/Simon