Re: unistd.h include in stringprep.h

[Simon Josefsson]

Bruno Haible <address@hidden> writes:

>> The #include happens in an installed header file, so the gnulib module
>> wouldn't help here.
>
> Oh, I see. A file that contains public API of libidn.

Right.

>> However, the only reason we include unistd.h is in order to get ssize_t.
>
> ssize_t on MSVC 9 was handled in
> <http://lists.gnu.org/archive/html/bug-gnulib/2011-09/msg00200.html>.
>
> In summary, to get ssize_t defined, you need the 'ssize_t' module or
> - equivalent - an invocation of gt_TYPE_SSIZE_T.

I don't see how this resolves anything?  It resolves the problem inside
the project, but for any external code that #include's my header file,
they will need to define an ssize_t themselves which is sub-optimal.

Is there really no ssize_t anywhere in MSVC9?  There appears to be a
type SSIZE_T though:

http://msdn.microsoft.com/en-us/library/aa383751%28v=VS.85%29.aspx#SSIZE_T

I can find people using ssize_t in their MSVC++ code though:

http://social.msdn.microsoft.com/Forums/en-US/vcgeneral/thread/e712a103-03af-4f62-b456-f550ecb9bf07

>
>> GnuTLS generates its header files that contains this:
>> 
>> /* Get ssize_t. */
>> #ifndef HAVE_SSIZE_T
>> #define HAVE_SSIZE_T
>> /* *INDENT-OFF* */
>> @DEFINE_SSIZE_T@
>> /* *INDENT-ON* */
>> #endif
>> 
>> and then have a configure.ac check looking like this:
>> 
>> AC_CHECK_TYPE(ssize_t,
>>   [
>>     DEFINE_SSIZE_T="#include <sys/types.h>"
>>     AC_SUBST(DEFINE_SSIZE_T)
>>   ], [
>>     AC_DEFINE([NO_SSIZE_T], 1, [no ssize_t type was found])
>>     DEFINE_SSIZE_T="typedef int ssize_t;"
>>     AC_SUBST(DEFINE_SSIZE_T)
>>   ], [
>>     #include <sys/types.h>
>>   ])
>
> It is not safe to define a type like 'ssize_t' in a public header file
> of your library, because it will conflict with packages that want to
> define ssize_t for their internal purpose.

Hopefully those definitions will be compatible, otherwise there will
likely be breakage anyway.

But still, arguable I agree, this should probably be gnutls_ssize_t or
something, but that makes the code quite ugly.

I'm inclined to leave this alone until someone reports a real problem
with it, having conflicting ssize_t definitions seems so problematic
that I doubt it will be a problem in practice.

> The public API that uses ssize_t is:
>
>   extern IDNAPI uint32_t *stringprep_utf8_to_ucs4 (const char *str,
>                                                    ssize_t len,
>                                                    size_t * items_written);
>   extern IDNAPI char *stringprep_ucs4_to_utf8 (const uint32_t * str,
>                                                ssize_t len,
>                                                size_t * items_read,
>                                                size_t * items_written);
>   extern IDNAPI char *stringprep_utf8_nfkc_normalize (const char *str,
>                                                       ssize_t len);
>   extern IDNAPI uint32_t *stringprep_ucs4_nfkc_normalize (uint32_t * str,
>                                                           ssize_t len);
>
> Note that ssize_t values are only passed by value, not through pointers.
> Therefore it's easy. I see two possible solutions:
>
>   1) Use size_t instead of ssize_t. This is backward compatible at the
>      source code level, except for uses of the 4 functions via function
>      pointers, and is also backward compatible at the ABI level.

The functions accept negative lengths -1 to indicate zero terminated
strings.  Using a non-signed type for storing -1 feels wrong to me.

>   2) Use 'long' instead of ssize_t. This is backward compatible at the
>      source code level, except for uses of the 4 functions via function
>      pointers. For ABI level backward compatibility you need to use
>      versioning (such as
>        #define stringprep_utf8_to_ucs4 stringprep_utf8_to_ucs4_v2
>      and a .c file that provides a definition of
>        stringprep_utf8_to_ucs4
>      followed by
>        #undef stringprep_utf8_to_ucs4
>      and a definition of stringprep_utf8_to_ucs4; similarly for the other
>      functions).

There is also the third alternative:

3) Define a libidn_ssize_t with what libidn believes to be the
appropriate ssize_t type, and use that.

and of course the fourth

4) Define ssize_t with what libidn believes to be the appropriate
ssize_t type, and use that, which will collide if something else defines
a type conflicting ssize_t type.

All things considered, I tend to prefer 4) as it leads to the cleanest
code for sane architectures that has a ssize_t (which are the most
important ones to me) and introduces minimal code to deal with platforms
that lacks a ssize_t.

/Simon