A little cleanup for SVN 1283

Werner Koch wk at gnupg.org
Wed Mar 19 20:01:03 CET 2008


On Wed, 19 Mar 2008 16:05, gcrypt at dstoecker.de said:

> HAVE_CONFIG_H only means, that a file config.h can be included or

Right.  But you don't have a choice in libgcrypt.  There we demand that
the config.h as created by configure needs to there.  It is a part of
the code.  If you don't want that, you are on your own and then, if you
like, add @ifdef HAVE_CONFIG_H.

> whole system to a config.h. Actually this is exactly what I do under
> Windows. I have the definitions in the project configuration.

I am sorry, for you, but we use a different build system and as I have
said hundreds of times, building on Windows is not supported.  A sed or awk
script to add the #ifdef HAVE_CONFIG_H si straightforward to implement
but in that case it would be easier to provide a condig.h (even if empty).

> All this goes together with the Makefile.am changes.
> The "-I$(top_srcdir)/mpi" allows including mpi-files. This goes
> together with the removal of "../mpi/" below.

You are wrong.  The ../src is required for VPATH builds.  VPATH builds
are a Good Thing and actually required by the GNU standards and due to
"make distcheck".

> This is a normal C forward declaration, which is perfectly
> valid. Don't know the reason, where I required this. Probably Windows
> again. Maybe the reason is gone already.

It would clash with systems redefining msghdr using cpp tricks.

> See above. No absolute path, but instead of this proper include
> definitions. This ../mpi/ is an ugly fix for the fact, that mpi was
> not included in the include parts of Makefile.am.

The semantics are different with your change.

> Again the same. All of the makefile.am changes cleanup the build to
> use proper definitions. There is no longer any need for "../" paths,
> which makes different building easier.

Well, I want to use them. The directories are closely coupled and thus
it is better to express this with the file names.

> It is wrong. You don't release the memory. Usually this will do no
> harm, but a test must be correct. Errors in test must be fixed as any
> normal errors. Especially as the tests are very likely used as user
> implementation examples.

This is a regression tests.  By releasing the memory you change the test
and may not detect a regression.  Okay, this is a bit of a theoretical
argument but I currently don't have the resources to fix and test the
regression tests.  

> You will be happy about it, whenever memory tracking tests will be
> added (which is what I did with the tests).

These will be new tests.

> Well, same as above. Freeing memory only.

As told, the ac funmctions will go anyway.  They are seriously bugged.

>> Automake already handles this.  No need to add it to Makefile.am
>
> Then the problem lies elsewhere. Without this the docs do not build
> from scratch.

No problem here to bootstrap it. 

> That is not true. The file config.h defines the values to 0 when disabled.
> Maybe that behaviour changed during autoconf/automake development, but now
> USE_ELGAMAL is always defined.

You are right.  The configure tests are broken.  Probably we never build
without some algorithms for a long time.  I'll fix configure.

>>  -#include <sys/socket.h>
>>  -#include <sys/un.h>
>>  +/*#include <sys/un.h>*/
>>  +#include "gcrypt.h"
>>
>> Why?
>
> sys.un.h is not required. gcrypt.h does WIN32 handling for <sys/socket.h>.

POSIX requires sys/un.h as it defines struct sockaddr_un.  Windows is
irrelevant here.  If you want to run change the build system for
Windows, I suggest to provide an sys/un.h stub.

> I fixed bugs I found to get stuff working. I didn't care if these is
> legacy or not. Bug is bug. If it is removed in the future, who cares,
> but until then it is fixed.

First of all I am not sure whether this is the correct fix.  We have
identified several inconsistencies in ac module and found no way to
properly solve that without an API change.

Second, yes bugs are to be fixed.  But sometimes it is better not to fix
a bug if it would introduce a couple of new and unknown bugs.  A
maintainer's job is to overview the overall stability of a software and
not to fix bugs just because there is a bug.  In particular not if it is
in a part scheduled for removal.


Shalom-Salam,

   Werner



-- 
Die Gedanken sind frei.  Auschnahme regelt ein Bundeschgesetz.




More information about the Gcrypt-devel mailing list