Hi Saxon, thanks!
I think it would be extremeny useful to be abble to compile APR with BC5 (iirc it can be downloaded for free?). What about the project files? Did you build your own, or convert the MSVC ones?
Saxon wrote:
Hi,
The project I'm using APR for has to compile under both MS Visual C 6 and Borland C 5, and I've been able to get it to compile under BC without too many hassles. I'm not sure if there's any interest in making APR work with BC straight out of CVS, but in case there is, here's the problems I came across:
1.
In file_io\win32\open.c, there's this function:
APR_DECLARE(apr_status_t) apr_os_file_put(apr_file_t **file, apr_os_file_t *thefile, apr_int32_t flags, apr_pool_t *pool) { (*file) = apr_pcalloc(pool, sizeof(apr_file_t)); (*file)->pool = pool; (*file)->filehand = *thefile; (*file)->ungetchar = -1; /* no char avail */ (*file)->flags; return APR_SUCCESS; }
On this line:
(*file)->flags;
(Line 521), I get a 'Code has no effect' warning. I presume this should actually be:
(*file)->flags = flags;
This seems to be a bug, yes. Can you provide a patch?
I think it would be better to #ifdef on the actual symbol we want to define, not on the compiler type. It might also be a good idea to change the values used by APR right now; I think Borland's values are the correct ones.
?
2.
I get a whole lot of warnings like this:
signal.h(67): W8017 Redefinition of 'SIGUSR1' is not identical signal.h(68): W8017 Redefinition of 'SIGUSR2' is not identical
The signal.h in the warning is Borland's library signal.h. The reason it occurs is that MSVC's signal.h has this:
#define SIGINT 2 #define SIGILL 4 #define SIGFPE 8 #define SIGSEGV 11 #define SIGTERM 15 #define SIGBREAK 21 #define SIGABRT 22
While Borland's signal.h has this:
#define SIGINT 2 #define SIGILL 4 #define SIGFPE 8 #define SIGSEGV 11 #define SIGTERM 15 #define SIGUSR1 16 #define SIGUSR2 17 #define SIGUSR3 20 #define SIGBREAK 21 #define SIGABRT 22
So, the problem is that include\arch\win32\apr_private.h sets its own values for SIGUSR1 and SIGUSR2, then when <signal.h> is included afterwards, it produces a conflict under Borland. Besides the ones defined in MSVC's signal.h, I think the signals in apr_private.h are arbitrary? So this problem could be fixed by re-ordering the specified signals so that SIGUSR1 and SIGUSR2 have the same values as Borland, and then putting #ifndef __BORLANDC__ around them to stop their definition in Borland. So, something like this:
Definitely the second one.
==================================================================== --- include/arch/win32/apr_private.h.old Wed Mar 27 17:06:27 2002 +++ include/arch/win32/apr_private.h Wed Mar 27 17:11:48 2002 @@ -129,14 +129,17 @@ #define SIGBUS 7 /* 8 is used for SIGFPE on windows */ #define SIGKILL 9 -#define SIGUSR1 10 +#define SIGSTKFLT 10 /* 11 is used for SIGSEGV on windows */ -#define SIGUSR2 12 +#define SIGCHLD 12 #define SIGPIPE 13 #define SIGALRM 14 /* 15 is used for SIGTERM on windows */ -#define SIGSTKFLT 16 -#define SIGCHLD 17 +#ifndef __BORLANDC__ +/* SIGUSR1 and SIGUSR2 are 16 and 17 for Borland C */ +#define SIGUSR1 16 +#define SIGUSR2 17 +#endif #define SIGCONT 18 #define SIGSTOP 19 #define SIGTSTP 20 ====================================================================
3.
In apr_filepath_root() in file_io\win32\filepath.c, on line 147 (just after the #else matching the #ifdef NETWARE near the top of the function), there's this:
char seperator[2] = { (flags & APR_FILEPATH_NATIVE) ? '\\' : '/', 0};
Borland doesn't like this kind of initializer.
This could be fixed something like this:
==================================================================== --- file_io/win32/filepath.c.old Wed Mar 27 17:06:33 2002 +++ file_io/win32/filepath.c Wed Mar 27 17:29:03 2002 @@ -144,9 +144,17 @@ return APR_EINCOMPLETE;
#else +#ifndef __BORLANDC__ char seperator[2] = { (flags & APR_FILEPATH_NATIVE) ? '\\' : '/', 0}; +#else + char seperator[2] = { 0, 0}; +#endif const char *delim1; const char *delim2; + +#ifdef __BORLANDC__ + seperator[0] = (flags & APR_FILEPATH_NATIVE) ? '\\' : '/'; +#endif
if (testpath[0] == '/' || testpath[0] == '\\') { if (testpath[1] == '/' || testpath[1] == '\\') { ====================================================================
Which means for non-Borland it's unchanged, or it could be changed for all compilers, like this:
As a general rule, one should avoid #ifdefs if at all possible; if not, check for specific features, not specific tools/versions/wahtever.
==================================================================== --- file_io/win32/filepath.c.old Wed Mar 27 17:06:33 2002 +++ file_io/win32/filepath.c Wed Mar 27 17:29:34 2002 @@ -144,9 +144,11 @@ return APR_EINCOMPLETE;
#else - char seperator[2] = { (flags & APR_FILEPATH_NATIVE) ? '\\' : '/', 0}; + char seperator[2] = { 0, 0}; const char *delim1; const char *delim2; + + seperator[0] = (flags & APR_FILEPATH_NATIVE) ? '\\' : '/';
if (testpath[0] == '/' || testpath[0] == '\\') { if (testpath[1] == '/' || testpath[1] == '\\') {
====================================================================
4.
No idea about this ... [snip]
5.
Or this. [snip]
Ugh. Stupid compiler should realize it's comparing with a constant, and that the comparison is well defined.6.
In file_io\win32\filedup.c, line 119, ie this:
if (stdhandle != -1) {
I get a 'Comparing signed and unsigned values' warning. This is because the stdhandle is a DWORD, which is an unsigned long. This could be fixed with this:
-1 on this change.
==================================================================== --- file_io/win32/filedup.c.old Wed Mar 27 17:06:33 2002 +++ file_io/win32/filedup.c Wed Mar 27 18:08:18 2002 @@ -96,7 +96,7 @@ #ifdef _WIN32_WCE return APR_ENOTIMPL; #else - DWORD stdhandle = -1; + DWORD stdhandle = (DWORD)-1; HANDLE hproc = GetCurrentProcess(); HANDLE newhand = NULL; apr_int32_t newflags; @@ -116,7 +116,7 @@ stdhandle = STD_INPUT_HANDLE; }
- if (stdhandle != -1) { + if (stdhandle != (DWORD)-1) { if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand, 0, TRUE, DUPLICATE_SAME_ACCESS)) {
====================================================================
7.
In file_io\win32\open.c, in res_name_from_filename(), on line 204, ie this:
if (rv = apr_conv_utf8_to_ucs2(file, &n, wfile + r, &d)) {
I get a warning that rv is assigned a value which is never used, so I imagine the 'rv = ' can be removed, and also the definition of 'apr_status_t rv;' on line 180.
Seems that way to me, too.
8.
In network_io\win32\sockets.c, at the top of apr_socket_create(), on line 123, ie this:
int downgrade = (family == AF_UNSPEC);
I get a warning that downgrade is assigned a value which is never used. This *is* actually used in the function, but inside a #if APR_HAVE_IPV6 / #endif (and APR_HAVE_IPV6 is 0 in apr.hw). So, I imagine that the definition of downgrade should also be wrapped in a #if APR_HAVE_IPV6 / #endif.
9.
In mmap\win32\mmap.c, in mmap_cleanup(), I get a warning that rv is assigned a value that is never used. This is referring to the rv on line 69, ie this:
apr_status_t rv = 0;
Which isn't necessary because the cases which assign something to rv redefine it in their own compound block.
Well, in this case it's a question of being on the safe side.
Those should be fixed. int<->unsigned is not a problem; int->short is ... well, on a LE machine it actually isn't ...10.
I get a bunch (14) of 'Suspicious pointer conversion' warnings, which seem to be for things like passing a pointer to a signed int to a function taking a pointer to an unsigned int, or vice versa. Or, passing a pointer to an int to a function expecting a pointer to a short.
11.
In include\arch\win32\misc.h, line 226, ie the last line of this:
APR_DECLARE_LATE_DLL_FUNC(DLL_WINBASEAPI, BOOL, WINAPI, GetFileAttributesExA, 0, ( IN LPCSTR lpFileName, IN GET_FILEEX_INFO_LEVELS fInfoLevelId, OUT LPVOID lpFileInformation), (lpFileName, fInfoLevelId, lpFileInformation));
I get a lot (26) of 'Cannot create pre-compiled header: initialized data in header' warnings.
12.
In include\arch\win32\atime.h, line 78, ie the definition of FileTimeToAprTime() function, I get 3 'Cannot create pre-compiled header: code in header' warnings.
All I can say is, don't use precompiled headers 'cause they're broken.
Probably should have an extra pair of parenthesis around the condition. Again, patches are welcome ...13.
I get a lot (31) of 'Possibly incorrect assignment' warnings, which is for things like this:
if (x = some_function(y)) { etc... }
I agree with -rch and -pch, perhaps -sus (although see my comment above), but not -pin, because it can be easily fixed.14.
I get a few (6) 'Unreachable code' warnings.
For points 10, 11, 12, 13, and 14, I imagine rather than modifying the code, it would be better to just disable the warnings. This can be done with the following:
==================================================================== --- include/apr.hw.old Wed Mar 27 17:06:25 2002 +++ include/apr.hw Wed Mar 27 19:10:21 2002 @@ -79,6 +79,7 @@ extern "C" { #endif
+#ifndef __BORLANDC__ /* disable or reduce the frequency of... * C4057: indirection to slightly different base types * C4075: slight indirection changes (unsigned short* vs short[]) @@ -89,6 +90,13 @@ * C4514: unreferenced inline function removed */ #pragma warning(disable: 4100 4127 4201 4514; once: 4057 4075 4244) +#else +/* disable warnings for Borland C */ +#pragma warn -sus /* suspicious pointer conversion */ +#pragma warn -pch /* cannot create pre-compiled header */ +#pragma warn -pia /* possibly incorrect assignment */ +#pragma warn -rch /* unreachable code */
+#endif
#define APR_INLINE __inline #define APR_HAS_INLINE 1
====================================================================
cya!
Saxon Druce ([EMAIL PROTECTED])
-- Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
