apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko ─îibej <br...@xbc.nu>
Subject Re: Borland C Builder 5
Date Wed, 27 Mar 2002 19:49:07 GMT
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?

>
>
>?
>
>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:
>
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.


>
>
>====================================================================
>--- 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:
>
Definitely the second one.
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]

>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:
>
Ugh. Stupid compiler should realize it's comparing with a constant, and 
that the comparison is well defined.
-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.

>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.
>
Those should be fixed. int<->unsigned is not a problem; int->short is 
... well, on a LE machine it actually isn't ...

>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.


>13.
>
>I get a lot (31) of 'Possibly incorrect assignment' warnings, which is for
>things like this:
>
>if (x = some_function(y)) {
>    etc...
>}
>
Probably should have an extra pair of parenthesis around the condition. 
Again, patches are welcome ...

>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 */
>
I agree with -rch and -pch, perhaps -sus (although see my comment 
above), but not -pin, because it can be easily fixed.

>+#endif
>
> #define APR_INLINE __inline
> #define APR_HAS_INLINE         1
>
>====================================================================
>
>cya!
>
>Saxon Druce (saxon@blocksoftware.com)
>


-- 
Brane ─îibej   <brane@xbc.nu>   http://www.xbc.nu/brane/




Mime
View raw message