[git] GCRYPT - branch, master, updated. libgcrypt-1.6.0-35-gcfc151b

[Dmitry Eremin-Solenikov]

On Fri, Jan 17, 2014 at 4:44 PM, Werner Koch <wk at gnupg.org> wrote:
> On Fri, 17 Jan 2014 12:36, dbaryshkov at gmail.com said:
>
>> into generated gpg-error.h header file? It would be platform
>> dependent, of course,
>
> Right and that is what needs to be avoided.  gpgrt shall replace
> platform dependent code in Libgcrypt, GPGME, and GnuPG.  It makes more
> sense to do have the platform dependent code at just one place.  In
> particular a future 64 bit Windows implementation of GnuPG will need it
> (e.g. sizeof(HANDLE) > sizeof(int))

I fully understand and support your idea of handling platform-dependent code
in one place. I suggest to have a platform-independent API, but with platform
dependent ABI. Applications using gpgrt won't be caring about exact _internals_
of gpgrt_lock_t (or GPGRT_LOCK_INITIALIZER). They should use those identifiers
in the program code as black boxes.

>
>> but existing 'dumped' mutexes also look platform dependent (and very fragile).
>
> I can't see how this is fragile.  Alignment requirements?  The test case
> should exhibity this soon.

Alignment, contents, libc/libpthread versioning, etc.

> Not exposing the internals also allows to change the internal
> implementation; for example from the fork problematic pthread mutexes to
> the cleaner semaphores.

Even if we put exact implementation to the header, we still can change
it later - provided we have proper structure versioning. You did
this in the past with thread CBS structure, so I see no problem with that.

Consider Linux kernel code. You have spinlock_t definition right in the headers.
Every file wanting to work with spinlocks, has to include that. However nobody
(well, except those who really have to) bothers to look inside it.

>> E.g. in it's current state gpg-error.h is definitely not multi-platform safe.
>
> As with several other header files, they are platform dependent.  We
> can't use configure macros in a public header.

You are correct about configure macros. However we can have one 'threading'
part for all posix systems.

>> And last BTW: if you care about ABI, most probably '_vers' field should be moved
>> to the beginning of the gpgrt_lock_t structure - thus it can be safely
>
> Breaks alignment requirements.  Thus it is easier to move it to the end.
> Okay, I could add another union or swap the union and the struct.  Is it
> worth the trouble?

It definitely is. Consider gpgrt switching from pthread to some 'semaphores'.
And let's suppose that sizeof(semaphore) > sizeof(pthread_mutex_t).
Thus version checking code will become completely useless - if you pass
old structure to new code, you risk doing 'after allocated block' access.
If you pass new structure to old code, it will face random garbage in the
_vers field (and by chance that garbage might have 1 in the right place).

>
>> And one would probably need the 'mutex type' magic value - because
>> otherwise one can (theoretically) build gpg-error library with posix header
>> for win32 and this will be unnoticed (if _vers fields fall to the same place).
>
> That has always been the case.  The configure macro actually prints a
> warning if you use a wrong one.

This

>
> Thanks for the comments.  I'll change the build systems to allow the
> inclusion of pre-generated lock objects for cross-compiling.

I think this defeats the idea of encapsulation of information about
pthread_mutex_t
structure in the libc headers. Teoretically one would have to have a special
copy of 'lock object' for each CPU-OS-libc combination. Does it worth trouble?


-- 
With best wishes
Dmitry