cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: CMake build patch - updated version.

From: tetetest tetetest <tetetest_at_rambler.ru>
Date: Fri, 02 Apr 2010 12:10:52 +0400

* Bill Hoffman <bill.hoffman_at_kitware.com> [Thu, 01 Apr 2010 12:01:35
-0400]:

> > I met the issue when I tried to add CURL_PULL_WS32TCPIP_H into
> curlbuild.h.
> > The problem is that it includes <winsock2.h>.
> >
>
> OK, why did you add CURL_PULL_WS32TCPIP_H ?

I took curlbuild.h.in as a known-good template. It has a section that
includes winsock2.h and ws32tcpip.h when CURL_PULL_WS32TCPIP_H
was defined:

/* Configure process defines this to 1 when it finds out that system */
/* header file ws2tcpip.h must be included by the external interface. */
#undef CURL_PULL_WS2TCPIP_H
#ifdef CURL_PULL_WS2TCPIP_H
# ifndef WIN32_LEAN_AND_MEAN
# define WIN32_LEAN_AND_MEAN
# endif
# include <windows.h>
# include <winsock2.h>
# include <ws2tcpip.h>
#endif

So I added this piece into curlbuild.h.cmake and it caused problems
with VS2008.

After consulting the other "known-good" example of curlbuild.h - the
distributed curlbuild.h.dist - I found that socklen_t is 'int' there,
and there are no attempts to include <winsock2.h>. So, in my
patch I did the same.

Perhaps I should have explained this more thoroughly in my post.

> > If you include the files the other way round, it works. The problem
is,
> > you cannot guarantee the order of #includes.
> >
> > Anyway, there is no problem with socklen_t in my patch.
> >
> Not now, but if windows or some compiler ever changes socklen_t it
could
> break. Seems much safer to use the system define if there is one.

This is exactly what I thought when trying to add
CURL_PULL_WS2TCPIP_H into curlbuild.h. Alas,
it is not as straightforward as it sounds.

Look:
1. You cannot use 'socklen_t' unless you have included <winsock2.h>
    which defines it.
2. You cannot include <winsock2.h> after the application has
    included <winsock.h>. They do not live together well. By the
   way, curl has met this problem before; see curl/curl.h:

#if defined(WIN32) && !defined(_WIN32_WCE) && !defined(__GNUC__) && \
  !defined(__CYGWIN__) || defined(__MINGW32__)
#if !(defined(_WINSOCKAPI_) || defined(_WINSOCK_H))
/* The check above prevents the winsock2 inclusion if winsock.h already
was
   included, since they can't co-exist without problems */
#include <winsock2.h>
#include <ws2tcpip.h>
#endif
#else

So, for now we cannot use socklen_t safely.

> > Yes, without including <sys/types.h> you cannot use socklen_t on
> > OpenSolaris.
> > On Linuxes or OpenBSD, you need not include it (at least on those
where
> > I can check the build).
> >
> > I admit that my way of setting CURL_PULL_SYS_TYPES_H is a bit
aggressive,
> > but it doesn't cause problems. I hope it fixes more than it breaks.
> >
> > Of course, adding a new test for it would be the best solution. But
we
> > can do it later.
> It would be helpful for me to see the problem before fixing it.

Hmmm... I am telling this from memory: on OpenSolaris, I got a
compilation
error in curlbuild.h for this line:

/* Data type definition of curl_socklen_t. */
typedef CURL_TYPEOF_CURL_SOCKLEN_T curl_socklen_t;

The error message said something like "socklen_t does not name a type".
If you need an exact message, I can reproduce the error today in the
evening
(but my Internet conneciton is down for a while, so I will be able to
reply
only tomorrow).

> > 1. If CMake is used to build curl in-source, curlbuild.h is not
renewed
> > because it
> > already comes with the distibution. This causes problems on OSes
> > that are not handled with the distributed curlbuild.h.
>
> CMake should overwrite the file because it will see there are changes
> right?

Not quite.
CMake command CONFIGURE_FILE() does not overwrite existing files.
This is not documented, but it is the way CMake works.

Besides, this command does not imply any dependency tracking. I mean
that
the resulting curlbuild.h is not in any dependency list of any target,
unless you add it into the list of sources...

Hmmm...

Another improvement idea, don't you think?

> > 2. If CMake is used to build curl out-of-source, then curl/curl.h
> > includes the wrong
> > header (again, the one that comes with the distribution). Look:
> >
> > curl/curl.h:
> > #include "curlver.h" /* libcurl version defines */
> > #include "curlbuild.h" /* libcurl build definitions !!! Note the
> > quotes around the filename */
> > #include "curlrules.h" /* libcurl rules enforcement */
>
> I am not seeing this, I am pretty sure my build is using the correct
> file....

Are you building from git sources or from an unpacked tgz distribution?
In git, the "non-configure" curlbuild.h is named "curlbuild.h.dist", so
it is
not included by curl.h and there are no clashes.

To check that curl.h includes the right file, you can do the following:

1. Copy curlbuild.h.dist into curlbuild.h in source tree.
2. Add an #error "Boom!" directive to curlbuild.h.

If the wrong file gets included, you will see an error.

--tetetest tetetest.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2010-04-02