portability patch

[pogma]

Hi, Building pidgin-2.2.2 on solaris2.6-2.10, hpux11.00-11.23, aix5.1-5.3, osf5.1, irix6.5 and red hat linux7.1 to rhel-5, we had some issues.

With the attached patch it works on all the above platforms.

Not included in the patch, because we scripted it, is a patch to add:

#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

to the top of every .c file, prior to any other #include statements. Without this, compilation failed on AIX.

The changes to delay initialization of GnuTLS are there because libgcrypt will exit the application on those systems without /dev/random or other entropy gathering socket source. Not good if you just want to use, e.g. AIM.

The configure alloca check is needed by getopt.c.

We changed

char s[2] = {erasechar(), 0};

to

char s[2];
s[0] = erasechar();
s[1] = 0;

Because of compiler complaints that the initializers must be constants.

Similar changes in a lot of places.

static struct _node root = {.ref = 1, .flags = 0};

This is c99, we have a number of c89 only compilers, so it became (ick):

static struct _node root = {
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
1,
0};

There were a few places that we had to change dot initializers.

Some compilers complained until we put 'typedef enum' after the enum involved.

osf segfaults using the forked dns resolver, forced it to use the blocking version.

A couple of variable declarations in the middle of blocks were changed.

Use inet_ntoa if there is no inet_ntop

"fixed" and aix infinite loop in the signal handler.

[pogma]

Build fixes for pidgin-2.2.2

[deryni]

Please split this patch up into its component pieces, that is into individual patches based on fix type.

[sadrul]

I don't like the changes to init array/struct individually. I hated it when we went ahead and removed the *useful* c99 features (named initialization) from our code. It will be extremely unfortunate if we have to litter the code with this mess now.

[deryni]

We need to decide if pidgin is going to require C99 or not. If it is then we can leave the named initializers, if not they have to go.

[pogma]

shorter patch without initializer changes

[rekkanoryo]

I vote to stick with C89. Named initializers may be nice, but there is nothing in C99 that justifies making our source not build on systems that don't have C99 compilers.

[sadrul]

We should use named initializers. It's easier to follow the code, when looking at it. There has been releases with them in the code, and the current stuff in jabber/parser.c has been there for over a year, without any bug reports about them. To me, it illustrates that there aren't any/enough people trying to build pidgin with c99-incompatible compilers. Trying to stick to c89 for aesthetic reasons alone is pointless.

[rlaager]

I don't think sticking to c89 is for aesthetic reasons... I think switching to c99's named initializers would be.

I did a *QUICK* read-through of this patch... The biggest concern I have right now is with the GnuTLS changes. I don't see why that was done. If a system has no entropy gathering device, why not just disable SSL support at compile-time? Either way, libgcrypt should be fixed to avoid calling exit() and instead return failure for the application to handle.

See also: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=412408

[pogma]

Replying to rlaager:

I did a *QUICK* read-through of this patch... The biggest concern I have right now is with the GnuTLS changes. I don't see why that was done. If a system has no entropy gathering device, why not just disable SSL support at compile-time? Either way, libgcrypt should be fixed to avoid calling exit() and instead return failure for the application to handle.

This was reported to the libgcrypt maintainers when we had the same issue with curl http://curl.haxx.se/mail/archive-2007-04/0099.html http://lists.gnupg.org/pipermail/gcrypt-devel/2007-April/001124.html

The systems that do not have an OS supplied EGD socket can run PRNGD, but it is possible that the daemon is not running, or is running with the socket at a different path than was compiled into libgcrypt. This is no reason to not be able to run pidgin and chat to people using AIM or another non-TLS using protocol, in my opinion.

[rlaager]

I think parts of this are ready to commit and other parts can be dropped entirely:

The libgcrypt guys don't seem interested in changing this. I don't like that they're calling abort() (instead of exit() like they were at the time of previous comments), but that's the way it is. That said, why are you compiling in SSL support for machines with no way of using it? I think I'd rather leave those changes out.

Not that it's likely to conflict with anything, but we should use something else instead of use_fork.

HAVE_STRUCT_SOCKADDR_SA_LEN has an extra underscore after HAVE in the configure script.

The changes to gtkmain.c should be obsoleted by newer GStreamer versions, as the comment in gtkmain.c says now.

What real name would you like me to use for COPYRIGHT (if you're not already listed)? What e-mail address should I use for the commit?

The config.h at the top of every file seems like something that needs more investigation. Do you know why that was required?

[pogma]

Replying to rlaager:

I think parts of this are ready to commit and other parts can be dropped entirely: The libgcrypt guys don't seem interested in changing this. I don't like that they're calling abort() (instead of exit() like they were at the time of previous comments), but that's the way it is. That said, why are you compiling in SSL support for machines with no way of using it? I think I'd rather leave those changes out.

For systems that do not have a native /dev/random, we rely on a pseudo random number generator PRNGD to provide some randomness, however, it is possible that the daemon is not running, and also possible that the pidgin user on those systems has no need of ssl. However, accepting/rejecting bits of the patch is entirely up to you.

Not that it's likely to conflict with anything, but we should use something else instead of use_fork.

Yes. Why I used is beyond me, I *know* that double underscore bracketed macros are reserved for the OS/compiler :(

HAVE_STRUCT_SOCKADDR_SA_LEN has an extra underscore after HAVE in the configure script.

Indeed it does, so the code always assumed it was missing on our systems for that build. Oh well, proves that it works :-)

The changes to gtkmain.c should be obsoleted by newer GStreamer versions, as the comment in gtkmain.c says now.

Ok.

What real name would you like me to use for COPYRIGHT (if you're not already listed)? What e-mail address should I use for the commit?

pogma@…. My real name is Peter O'Gorman, but the copyright belongs to my employer, The Written Word, Inc.

The config.h at the top of every file seems like something that needs more investigation. Do you know why that was required?

I believe that it was because none of our AIX systems would compile pidgin otherwise. config.h includes macros for, for example, large file support, that must be set before any system headers are included.

Note that the requirement to have config.h as the first include in all source files is listed here: http://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers "The package should `#include' the configuration header file before any other header files, to prevent inconsistencies in declarations (for example, if it redefines const)."

If I recall correctly, pidgin #includes some headers, then ends up #including config.h and that caused xlc to stop the build with an error when another system header was included.

Thank you very much for looking into this, I had thought it forgotten.

[sadrul]

I believe all/most files include "internal.h" in the beginning, which includes config.h at the very top. Should/does that not work?

[pogma]

Replying to sadrul:

I believe all/most files include "internal.h" in the beginning, which includes config.h at the very top. Should/does that not work?

It should. There must still have been some source files without this though, or it would not have failed. I will try to make the time to build from your development sources this week on AIX to find the files that are missing this.

[rlaager]

Thanks for the patch!

I committed most of the short patch, subject to my previous comment. The MSN directconn.c change in the short patch looks suspect to me as well so I didn't commit that.

I looked at the longer patch. Can you explain the libpurple/cmds.h changes, please?

You also have a change with _XOPEN_SOURCE_EXTENDED in a couple places in libgnt. Is that still necessary for you? Can you explain why you dropped the "#if defined(APPLE) defined(unix)"?

I would love it if you could check out a copy of MTN head and see what changes are needed to make it compile.

[pogma@…]

(In [e7d6a0a271d8fe8435f200e0f1ec123053e57493]):
A patch from Peter O'Gorman at The Written Word, Inc. to fix various portability issues. These changes seemed reasonable, even though I wasn't able to test or verify them all in particular. Hopefully we don't break anything on another OS. Refs #3798

[trac-robot]

This ticket was closed automatically by the system. It was previously set to a Pending status and hasn't been updated within 14 days.