Hi, ----- Original Message ----- From: "Branko Äibej" <[EMAIL PROTECTED]> Sent: Thursday, March 28, 2002 3:49 AM
> I think it would be extremeny useful to be abble to compile APR with BC5 > (iirc it can be downloaded for free?). I think the free BC5 download is just the command-line compiler, not the whole IDE (which I have). > What about the project files? Did > you build your own, or convert the MSVC ones? I built my own. I created a new project, added all the files to it which are in the MSVC project, then added some include dirs and defines, and it went from there. > >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? Sure, here it is: ==================================================================== --- file_io/win32/open.c.old Wed Mar 27 17:06:33 2002 +++ file_io/win32/open.c Thu Mar 28 09:06:02 2002 @@ -177,7 +177,6 @@ apr_wchar_t *wpre, *wfile, *ch; apr_size_t n = strlen(file) + 1; apr_size_t r, d; - apr_status_t rv; if (apr_os_level >= APR_WIN_2000) { if (global) @@ -201,7 +200,7 @@ wfile = apr_palloc(pool, (r + n) * sizeof(apr_wchar_t)); wcscpy(wfile, wpre); d = n; - if (rv = apr_conv_utf8_to_ucs2(file, &n, wfile + r, &d)) { + if (apr_conv_utf8_to_ucs2(file, &n, wfile + r, &d)) { return NULL; } for (ch = wfile + r; *ch; ++ch) { @@ -518,7 +517,7 @@ (*file)->pool = pool; (*file)->filehand = *thefile; (*file)->ungetchar = -1; /* no char avail */ - (*file)->flags; + (*file)->flags = flags; return APR_SUCCESS; } ==================================================================== This fixes this bug, and also the 'rv assigned a value but not used' mentioned later. > >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. Ok, sure. How would I go about doing the #ifdef on the symbol? I can't do this: #ifndef SIGUSR1 #define SIGUSR1 16 #endif etc because this is done before the compiler's signal.h is included, so SIGUSR1 will always be undefined. Unless I had apr_private.h #include <signal.h> itself, first? > >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. Should I submit a patch for this one? > >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. I'm not sure what you mean? The function goes roughly: static apr_status_t mmap_cleanup(void *themmap) { apr_status_t rv = 0; if (something) { if (!some_func(something)) { apr_status_t rv = apr_get_os_error(); ... return rv; } } if (something_else) { if (!some_other_func(something_else)) { apr_status_t rv = apr_get_os_error(); ... return rv; } } return APR_SUCCESS; } So in the 2 cases which return rv, they declare their own 'apr_status rv' in their local scope, setting it to apr_get_os_error(), overriding the original 'apr_status_t rv = 0' at the top of the function. So that first rv is never used? > >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 ... Ok sure, I'll look into each of these individually when I get some more time. > >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 ... For Borland, to suppress the warning you need: if ((x = some_function(y)) != 0) { etc... } If you don't think that's too much, I'll submit patches changing them all to that. > >==================================================================== > >--- 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. Ok, here's a new patch with those two, to make it easier: ==================================================================== --- include/apr.hw.old Wed Mar 27 17:06:25 2002 +++ include/apr.hw Thu Mar 28 09:31:13 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,11 @@ * C4514: unreferenced inline function removed */ #pragma warning(disable: 4100 4127 4201 4514; once: 4057 4075 4244) +#else +/* disable warnings for Borland C */ +#pragma warn -pch /* cannot create pre-compiled header */ +#pragma warn -rch /* unreachable code */ +#endif #define APR_INLINE __inline #define APR_HAS_INLINE 1 ==================================================================== cya, Saxon Druce ([EMAIL PROTECTED])