jabberd-2.1.9 source distribution contain platform specifi characters and C prototype mismatches

[plmogan]

In jabberd-2.1.9, many C header and source contains the MSDOS \r\n instead of just \n, thus choking the freely available SUN Studio 11 C compiler. We have to use the SUN Solaris dos2unix -ascii -iso in a shell script to clean up these extra end of line characters to proceed. Then, we ran into ANSI C prototype mismatching problems, as shown below:

"c2s.c", line 218: warning: argument #2 is incompatible with prototype:
        prototype: pointer to const unsigned char : "../util/util.h", line 251
        argument : pointer to char
"c2s.c", line 236: warning: argument #4 is incompatible with prototype:
        prototype: pointer to const unsigned char : "../util/util.h", line 259
        argument : pointer to char
"c2s.c", line 277: warning: argument #1 is incompatible with prototype:
        prototype: pointer to const char : "/usr/include/iso/string_iso.h", line 69
        argument : pointer to const unsigned char
"c2s.c", line 277: warning: argument #2 is incompatible with prototype:
        prototype: pointer to const char : "../util/nad.h", line 158
        argument : pointer to const unsigned char
"c2s.c", line 445: operands have incompatible types:
         pointer to const unsigned char ":" pointer to char
"c2s.c", line 552: warning: argument #1 is incompatible with prototype:
        prototype: pointer to const char : "/usr/include/iso/string_iso.h", line 64
        argument : pointer to unsigned char
"c2s.c", line 891: warning: argument #2 is incompatible with prototype:
        prototype: pointer to char : "unknown", line 0
        argument : pointer to unsigned char
"c2s.c", line 941: warning: argument #3 is incompatible with prototype:
        prototype: pointer to const char : "../sx/sx.h", line 172
        argument : pointer to const unsigned char
cc: acomp failed for c2s.c
make[2]: *** [c2s-c2s.o] Error 2
make[2]: Leaving directory `/src/network/sources/jabberd-2.1.9/c2s'

The configure generated Makefiles for both c2s and sm seems to assume that everyone is using GCC, thus the GCC specific -export-dynamic is used in Makefiles for both. This again chocks the SUN Studio C compiler. Each time we build jabberd2 on SUN UltraSPARC Solaris 10 machines, we have to manually fix these. 2.1.6 built fine, but with the new prototype mismatch errors with 2.1.9, we will have to spend some time to look and see.

[smoku]

(In [298]) Changed DOS line endings to UNIX line endings. Refs #111

[smoku]

(In [299]) Reverted r298. Refs #111

[smoku]

(In [300]) Changed DOS line endings to UNIX line endings. Refs #111

[smoku]

-export-dynamic is a libtool option which should be converted to platform specific options by libtool helper. Does it work wrong?

[smoku]

Please describe what exact changes you need to make to make it compile. Or a diff would be nice.

(I do not have Solaris machine at hand to debug it.)

[plmogan]

Replying to smoku:

Please describe what exact changes you need to make to make it compile. Or a diff would be nice. (I do not have Solaris machine at hand to debug it.)

Hello,

Judging from my experience building jabberd2 (from 2.0sx to 2.1.x) and "tolerated" this defect for years, I would say it does work incorrectly.

Regards,

--Peter

[plmogan]

Replying to plmogan:

Replying to smoku:

Please describe what exact changes you need to make to make it compile. Or a diff would be nice. (I do not have Solaris machine at hand to debug it.)

Hello,

Thanks for your attention to this ticket. Below is what I have done to get most of the source to compile with the following settings:

Before running ./configure, do the following:

in tcsh

setenv CPPFLAGS '-I/usr/local/include -I/usr/local/include/openssl -I/usr/local/include/mysql' setenv LDFLAGS '-L/usr/local/lib -R/usr/local/lib -L/usr/local/lib/mysql -R/usr/local/lib/mysql -s'

As you can tell, we build and package everything we use. Then, follow the hints given in http://jabberd2.xiaoka.com/wiki/SunOSInstallation not only for sm, but also for c2s as well. I mentioned explicitly the --export-dynamic GCC specific option which chocks the much superior SUN Studio 11/12 compilers, whichare also freely available to various flavors of Linux.

Please note, I am not knocking on CCC. I only state a easily verifiable fact that SUN's free compilers produces simply much superior executables. A fact that even Apache folks acknowledges openly in their doc.

After receiving a lot errors like the following:

make[2]: Entering directory `/src/network/sources/jabberd-2.1.9/c2s' source='c2s.c' object='c2s-c2s.o' libtool=no \

DEPDIR=.deps depmode=none /bin/bash ../depcomp

\

/opt/SUNWspro/bin/cc -DHAVE_CONFIG_H -I. -I..

-DCONFIG_DIR=\"/usr/local/etc\" -DLIBRARY_DIR=\"/usr/local/lib/jabberd\" -I/usr/local/include -I/usr/local/include/openssl -I/usr/local/include/mysql -xO3 -xbuiltin -xdepend=yes -xprefetch=auto,explicit -xarch=v8plusa -xcache=16/32/1:256/64/4 -xchip=ultra2e -c -o c2s-c2s.o test -f 'c2s.c' || echo './'c2s.c "c2s.h", line 21: warning: invalid white space character in directive "c2s.h", line 22: warning: invalid white space character in directive "c2s.h", line 23: warning: invalid white space character in directive "c2s.h", line 25: warning: invalid white space character in directive "c2s.h", line 27: warning: invalid white space character in directive "../mio/mio.h", line 21: warning: invalid white space character in directive [... many more ...] I immediately became suspicious. vi c2s/c2s.h immediately showed these familiar ^{M \r characters. My conjecture is that somehow the recent Microsoft Windows port introduced these codes, as whoever worked on such ports took the original sources and edited in Windows environment, overlooking the need to clean up the end of line characters are the porting was done.}

To prevent similar accidents in the future, perhaps a system that emulates what Apache and PHP people have done in this regard would be fruitful.

I then simply used a for file in *.[ch] do dos2unix -ascii -iso $file $file done in ech source directory (e.g. c2s, sm, router, resolver...) to clean out all files just in case. I didn't check each and every file. The above shell script is easy and fast.

I still haven't got time to investigate the prototype mismatch problem yet. A quick diff showed that there are substantial changes introduced in c2s.c from 2.1.6 to 2.1.9. Since 2.1.6 so far works fine for us, and since I am working on a pressing project now, I will have to do it some other time. If someone could fix these mismatches before I can, I would be more than happy to test build. That, I can squeeze out sometime.

Just a final reminder, for people who are curious about the excellent and freely available SUN Studio compiler suite, please see http://developers.sun.com/sunstudio/index.jsp

For people who don't have a computer running Solaris or the freely available OpenSolaris? (yes, you get source for this OS too), but use Linux instead, please see http://developers.sun.com/sunstudio/linux_index.html

I have years of experience using both GCC (I still do) and SUN Studio compiler suite. The later almost always generate bigger executables, but often much faster too. Given today's computers with large RAM, of course my preference is to build everything with SUN compilers. It also has much superior online documentation at http://docs.sun.com/ too.

Regards,

--Peter

[plmogan]

Introduction

I decided to take some time to try to figure out why recent jabberd-2.1.x (x > 6) fail to build with SUN Studio 11 compiler in SUN Solaris 11/06 on a UltraSPARC machine. BTW, I used GNU make 3.81 instead of the SUN Solaris bundled /usr/ccs/bin/make.

Observations

Below is what I found out so far, and the results of my preliminary investigation:

2.1.13 builts better. Now it only fails in c2s.c. if the c2s.c is replaced with its counterpart from 2.1.6 release, then the entire distribution builds fine. I haven't had time to try to actually run the resulted build. I only got time to do a quick /usr/local/bin/jabberd and didn't observe any crashes.

Findings

Afterwards, I decided to make a context diff of the c2s.c from 2.1.13 and 2.1.6, as shown below:

*** 2.1.13-c2s.c        Thu Aug  9 15:38:10 2007
--- 2.1.6-c2s.c Thu Aug  9 15:38:32 2007
***************
*** 78,86 ****
                  }
  
                  if(s->state >= state_STREAM && sess->jid != NULL)
!                     log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s] read error: %s (%d)", sess->fd->fd, jid_full(sess->jid), MIO_STRERROR(MIO_ERROR), MIO_ERROR);
                  else
!                     log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] read error: %s (%d)", sess->fd->fd, sess->ip, sess->port, MIO_STRERROR(MIO_ERROR), MIO_ERROR);
  
                  sx_kill(s);
                  
--- 78,86 ----
                  }
  
                  if(s->state >= state_STREAM && sess->jid != NULL)
!                     log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s] read error: %s (%d)", sess->fd->fd, jid_full(sess->jid), strerror(errno), errno);
                  else
!                     log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] read error: %s (%d)", sess->fd->fd, sess->ip, sess->port, strerror(errno), errno);
  
                  sx_kill(s);
                  
***************
*** 144,152 ****
                  return 0;
              
              if(s->state >= state_OPEN && sess->jid != NULL)
!                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s] write error: %s (%d)", sess->fd->fd, jid_full(sess->jid), MIO_STRERROR(MIO_ERROR), MIO_ERROR);
              else
!                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s. port=%d] write error: %s (%d)", sess->fd->fd, sess->ip, sess->port, MIO_STRERROR(MIO_ERROR), MIO_ERROR);
          
              sx_kill(s);
          
--- 144,152 ----
                  return 0;
              
              if(s->state >= state_OPEN && sess->jid != NULL)
!                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s] write error: %s (%d)", sess->fd->fd, jid_full(sess->jid), strerror(errno), errno);
              else
!                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s. port=%d] write error: %s (%d)", sess->fd->fd, sess->ip, sess->port, strerror(errno), errno);
          
              sx_kill(s);
          
***************
*** 198,206 ****
              break;
  
          case event_PACKET:
-             /* we're counting packets */
-             sess->packet_count++;
- 
              nad = (nad_t) data;
  
              /* we only want (message|presence|iq) in jabber:client, everything else gets dropped */
--- 198,203 ----
***************
*** 327,337 ****
  #ifdef HAVE_SSL
              /* drop packets if they have to starttls and they haven't */
              if((sess->s->flags & SX_SSL_STARTTLS_REQUIRE) && sess->s->ssf == 0) {
-                 log_debug(ZONE, "pre STARTTLS packet, dropping");
-                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] got pre STARTTLS packet, dropping", sess->s->tag);
- 
-                 sx_error(s, stream_err_POLICY_VIOLATION, "stanza sent before starttls");
- 
                  nad_free(nad);
                  return 0;
                  return 0;
              }
--- 324,329 ----
***************
*** 372,378 ****
  
              /* they sasl auth'd, so we only want the new-style session start */
              else {
!                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] SASL authentication succeeded: mechanism=%s; authzid=%s%s", sess->s->tag, &sess->s->auth_method[5], sess->s->auth_id, 
sess->s->ssf ? ", TLS negotiated" : "");
                  sess->sasl_authd = 1;
              }
  
--- 364,370 ----
  
              /* they sasl auth'd, so we only want the new-style session start */
              else {
!                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] SASL authentication succeeded: mechanism=%s; authzid=%s", sess->s->tag, &sess->s->auth_method[5], sess->s->auth_id);
                  sess->sasl_authd = 1;
              }
  
***************
*** 442,448 ****
          case action_CLOSE:
              log_debug(ZONE, "close action on fd %d", fd->fd);
  
!             log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] disconnect jid=%s, packets: %i", sess->fd->fd, sess->ip, sess->port, sess->jid?jid_full(sess->jid):"unbound"
, sess->packet_count);
  
              /* tell the sm to close their session */
              if(sess->active)
--- 434,440 ----
          case action_CLOSE:
              log_debug(ZONE, "close action on fd %d", fd->fd);
  
!             log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] disconnect", sess->fd->fd, sess->ip, sess->port);
  
              /* tell the sm to close their session */
              if(sess->active)
***************
*** 593,599 ****
                      return 0;
                  }
  
!                 log_write(c2s->log, LOG_NOTICE, "[%d] [router] read error: %s (%d)", c2s->fd->fd, MIO_STRERROR(MIO_ERROR), MIO_ERROR);
  
                  sx_kill(s);
                  
--- 585,591 ----
                      return 0;
                  }
  
!                 log_write(c2s->log, LOG_NOTICE, "[%d] [router] read error: %s (%d)", c2s->fd->fd, strerror(errno), errno);
  
                  sx_kill(s);
                  
***************
*** 625,631 ****
              if(errno == EWOULDBLOCK || errno == EINTR || errno == EAGAIN) 
                  return 0;
  
!             log_write(c2s->log, LOG_NOTICE, "[%d] [router] write error: %s (%d)", c2s->fd->fd, MIO_STRERROR(MIO_ERROR), MIO_ERROR);
          
              sx_kill(s);
          
--- 617,623 ----
              if(errno == EWOULDBLOCK || errno == EINTR || errno == EAGAIN) 
                  return 0;
  
!             log_write(c2s->log, LOG_NOTICE, "[%d] [router] write error: %s (%d)", c2s->fd->fd, strerror(errno), errno);
          
              sx_kill(s);

I started changing the c2s.c by introducing changes one by one. In so doing, however, I skipped the following:

*** 198,206 ****
              break;
  
          case event_PACKET:
-             /* we're counting packets */
-             sess->packet_count++;
- 
              nad = (nad_t) data;
  
              /* we only want (message|presence|iq) in jabber:client, everything else gets dropped */
--- 198,203 ----
***************
*** 327,337 ****
  #ifdef HAVE_SSL
              /* drop packets if they have to starttls and they haven't */
              if((sess->s->flags & SX_SSL_STARTTLS_REQUIRE) && sess->s->ssf == 0) {
-                 log_debug(ZONE, "pre STARTTLS packet, dropping");
-                 log_write(sess->c2s->log, LOG_NOTICE, "[%d] got pre STARTTLS packet, dropping", sess->s->tag);
- 
-                 sx_error(s, stream_err_POLICY_VIOLATION, "stanza sent before starttls");
- 
                  nad_free(nad);
                  return 0;
              }

Right after I introduced the following:

*** 442,448 ****
          case action_CLOSE:
              log_debug(ZONE, "close action on fd %d", fd->fd);
  
!             log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] disconnect jid=%s, packets: %i", sess->fd->fd, sess->ip, sess->port, sess->jid?jid_full(sess->jid):"unbound"
, sess->packet_count);
  
              /* tell the sm to close their session */
              if(sess->active)
--- 434,440 ----
          case action_CLOSE:
              log_debug(ZONE, "close action on fd %d", fd->fd);
  
!             log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] disconnect", sess->fd->fd, sess->ip, sess->port);
  
              /* tell the sm to close their session */
              if(sess->active)

the following error appeared:

Making all in c2s
make[2]: Entering directory `/src/network/sources/jabberd-2.1.13/c2s'
source='c2s.c' object='c2s-c2s.o' libtool=no \
        DEPDIR=.deps depmode=none /bin/bash ../depcomp \
        /opt/SUNWspro/bin/cc -DHAVE_CONFIG_H -I. -I..  -DCONFIG_DIR=\"/usr/local/etc\" -DLIBRARY_DIR=\"/usr/local/lib/jabberd\" -I/usr/local/include -I/usr/local/include/openssl -I/usr/local/include/mysql  -xO3 - -c -o c2s-c2s.o `test -f 'c2s.c' || echo './'`c2s.c
"c2s.c", line 215: warning: argument #2 is incompatible with prototype:
        prototype: pointer to const unsigned char : "../util/util.h", line 253
        argument : pointer to char
"c2s.c", line 233: warning: argument #4 is incompatible with prototype:
        prototype: pointer to const unsigned char : "../util/util.h", line 261
        argument : pointer to char
"c2s.c", line 274: warning: argument #1 is incompatible with prototype:
        prototype: pointer to const char : "/usr/include/iso/string_iso.h", line 69
        argument : pointer to const unsigned char
"c2s.c", line 274: warning: argument #2 is incompatible with prototype:
        prototype: pointer to const char : "../util/nad.h", line 164
        argument : pointer to const unsigned char
"c2s.c", line 437: operands have incompatible types:
         pointer to const unsigned char ":" pointer to char
"c2s.c", line 544: warning: argument #1 is incompatible with prototype:
        prototype: pointer to const char : "/usr/include/iso/string_iso.h", line 64
        argument : pointer to unsigned char
"c2s.c", line 883: warning: argument #2 is incompatible with prototype:
        prototype: pointer to char : "unknown", line 0
        argument : pointer to unsigned char
"c2s.c", line 933: warning: argument #3 is incompatible with prototype:
        prototype: pointer to const char : "../sx/sx.h", line 172
        argument : pointer to const unsigned char
cc: acomp failed for c2s.c
make[2]: *** [c2s-c2s.o] Error 2
make[2]: Leaving directory `/src/network/sources/jabberd-2.1.13/c2s'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/src/network/sources/jabberd-2.1.13'
make: *** [all] Error 2

Conclusions

Unlike what I initially suspected, the failer is not caused by prototype mismatches. According to Chapter 6 Transition to ISO C, the compiler's -Xa default actually accomodates K&R syntaxes (see 6.1.2). So, the problem is caused by the newly introduced constructs identified above.

I hope the above findings can help resolve this ticket.

Thanks,

--Peter

[plmogan]

Although I am still pressed by project pressure, I decided to stay late at work and took another look of the c2s.c compilation problem. I soon reached the conclusion that the SUN Studio 11 compiler doesn't like the conditional expression sess->jid?jid_full(sess->jid):"unbound", so I replaced it with an equalent if-else contruct. Please see the following context diff

# diff -c 2.1.13-c2s.c 2.1.13m-c2s.c
*** 2.1.13-c2s.c        Thu Aug  9 15:38:10 2007
--- 2.1.13m-c2s.c       Thu Aug  9 21:14:31 2007
***************
*** 442,449 ****
          case action_CLOSE:
              log_debug(ZONE, "close action on fd %d", fd->fd);
  
!             log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] disconnect jid=%s, packets: %i", sess->fd->fd, sess->ip, sess->port, sess->jid?jid_full(sess->jid):"unbound", sess->packet_count);
! 
              /* tell the sm to close their session */
              if(sess->active)
                  sm_end(sess);
--- 442,455 ----
          case action_CLOSE:
              log_debug(ZONE, "close action on fd %d", fd->fd);
  
!             char *cptr;
!             if (sess->jid)
!                 cptr = jid_full(sess->jid);
!             else
!                 cptr = "unbound";
!             
!             log_write(sess->c2s->log, LOG_NOTICE, "[%d] [%s, port=%d] disconnect jid=%s, packets: %i", sess->fd->fd, sess->ip, sess->port, cptr, sess->packet_count);
!             
              /* tell the sm to close their session */
              if(sess->active)
                  sm_end(sess);

Now the entire 2.1.13 distribution builds fine, despite lots of prototype mismatch warnings. I however have not got time to test the build yet. But at least this is a small progress. Perhaps someone else who has access to SUN Studio compilers (now 12 is available for free download to both Solaris and Linux) should give both the modified version and the original a try. I have a suspicion that the compiler that we use might have a bug :(

--Peter

[smoku]

The prototype mismatches are only warnings. These are also under GCC.

The problem is:

"c2s.c", line 437: operands have incompatible types:
         pointer to const unsigned char ":" pointer to char

I will commit a fix in a minute.

[smoku]

(In [345]) Operands incompatibility fix for Sun compiler. Fixes #111

[smoku]

If the problem persists, or if there are any other related issues, please feel free to reopen the ticket.