[prev in list] [next in list] [prev in thread] [next in thread]
List: apr-dev
Subject: Re: Borland C Builder 5
From: "Saxon" <saxon () saxon ! cx>
Date: 2002-03-28 1:29:20
[Download RAW message or body]
Hi,
----- Original Message -----
From: "Branko Čibej" <brane@xbc.nu>
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 (saxon@blocksoftware.com)
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic