apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: [PATCH] Support WINNT builds and initial WINCE support
Date Mon, 28 Jan 2002 16:44:24 GMT
From: "Mladen Turk" <mturk@mappingsoft.com>
Sent: Monday, January 28, 2002 6:56 AM


> > From: Mladen Turk [mailto:mturk@mappingsoft.com] 
> > 
> > Hi, all
> > 
> > The patch solves the NT only builds and the non-ANSI 
> > architectures. Since I have a working port of current APR for 
> > Windows CE 3.0+ to be able to start to include the needed 
> > changes, this patches are needed.
> > 
> > How it works:
> > 
> > Beside the current APR_HAS_UNICODE_FS, I've added the 
> > APR_HAS_ANSI_FS preprocessor flag. Additionaly there is 
> > compile time flag WINNT that forces to exclude all the non-NT 
> > code from the build. I've changed the OS enumeration with the 
> > LSB indicating if the OS supports UNICODE.

Mladen, changed a couple of things in committing your port; see below

there was no reason to go overboard on the os_version enumeration...
unless I'm missing something, the simply insertion of WIN_CE_3 should
have worked [see cvs HEAD].  Let me know what exactly is broken that
we need to revamp that.  Certainly, it's needed to accomplish Unicode.

I simplified the macro names so they are just a bit more legible.
Great approach - it's a patch I meant to write to speed up NT-only
builds, I'm glad this helps both NT and CE :)


> Fortgot one...
> Apr.hw


So here are my comments about your apr.hw changes proposed...


Index: apr.hw
===================================================================
RCS file: /home/cvspublic/apr/include/apr.hw,v
retrieving revision 1.82
diff -u -r1.82 apr.hw
--- apr.hw  10 Jan 2002 00:20:18 -0000  1.82
+++ apr.hw  28 Jan 2002 12:55:25 -0000
@@ -96,15 +96,14 @@
 #ifndef NOGDI
 #define NOGDI
 #endif
-#ifndef NONLS
-#define NONLS
-#endif
 #ifndef NOMCX
 #define NOMCX
 #endif
 #ifndef NOIME
 #define NOIME
 #endif
+
+#define APR_HAS_WINSOCK2    1

You use this below, but I'm unclear.  Where are you 'toggling' this?  Where are
we now supporting non-winsock2?  If CE is a single socket API - is it effectively
winsock or winsock2?  And what are the rest of the compatibility patches we have
to put it to support the old winsock?

I'm very uncomfortable in this territory, there we a number of good reasons for
dropping winsock support in Apache, and this could be a huge step backwards.

In any case, APR_HAS_WINSOCK2 should be a private internal detail, since it's
another symbol that must land in apr.hw/apr.h.in.  If we can't do so, it seems
like you were reaching for APR_HAVE_WINSOCK2_H as a symbol name.


@@ -112,16 +111,14 @@
  * winsock headers were excluded by WIN32_LEAN_AND_MEAN, so include them now
  */
 #define SW_HIDE             0
+#ifdef APR_HAS_WINSOCK2
 #include <winsock2.h>
 #include <mswsock.h>
+#else
+#include <winsock.h>
+#endif

Ick yuck.  See my longer comment above.


 #endif /* !_WINDOWS_ */
 
-#include <sys/types.h>
-#include <stddef.h>
-#include <stdio.h>
-#include <time.h>
-#include <process.h>
-#include <stdlib.h>

Ok... some of these seem a bit odd after their move, see my later comments.


@@ -175,6 +172,9 @@
 #define APR_HAVE_SYS_UIO_H      0
 #define APR_HAVE_SYS_WAIT_H     0
 #define APR_HAVE_UNISTD_H       0
+#define APR_HAVE_TIME_H         1
+#define APR_HAVE_STDDEF_H       1
+#define APR_HAVE_PROCESS_H      1
 
Ok, WinCE has no TIME/STDDEF/PROCESS.  Where do we say so [I didn't see it
in your patch.]  Such symbols cannot be added to apr.hw without being added
to apr.h.in.


@@ -223,6 +223,17 @@
 #if APR_HAVE_SYS_TYPES_H
 #include <sys/types.h>
 #endif
+#if APR_HAVE_TIME_H

presume you meant APR_HAVE_SYS_TYPES_H

+#include <stddef.h>
+#endif
+#include <stdio.h>

presume you meant to add an APR_HAVE_STDIO_H?

+#if APR_HAVE_TIME_H
+#include <time.h>
+#endif
+#if APR_HAVE_PROCESS_H
+#include <process.h>
+#endif
+#include <stdlib.h>

presume you meant to add an APR_HAVE_STDLIB_H?



@@ -236,9 +247,17 @@
 #define APR_HAS_DSO               1
 #define APR_HAS_SO_ACCEPTFILTER   0
 #define APR_HAS_UNICODE_FS        1
+#if defined(_WIN32_WCE) || defined(WINNT)
+#define APR_HAS_ANSI_FS           0
+#else
+#define APR_HAS_ANSI_FS           1
+#endif


I moved this ANSI symbol since the APR has to reason to share this symbol.
Every string coming into APR is char data, and the app is only interested 
if the strings must be UTF-8 [APR_HAS_UNICODE_FS], or not.  It doesn't
belong as a public symbol, local narrow charset code pages are the default
across APR.  All of this code now lives in misc.h.




 #define APR_HAS_USER              1
 #define APR_HAS_LARGE_FILES       1
 #define APR_HAS_XTHREAD_FILES     1
+#define APR_HAS_PIPES             1
+#define APR_HAS_STDIO             1
+


I'm confused, didn't see these symbols used in your patch.  Again, a change
that must happen to both apr.hw and apr.h.in.  Did you mean STDIO to be
APR_HAVE_STDIO_H?  And we don't want to proliferate symbols that aren't tested,
if you will need this 'soonish', wait until you have a patch that demonstrates
the need for the new symbols.






Mime
View raw message