You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Mladen Turk <mt...@apache.org> on 2004/10/06 15:52:43 UTC

Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

ake@apache.org wrote:
> ake         2004/10/06 06:33:29
> 
>   Modified:    modules/arch/win32 mod_win32.c
>                server/mpm/winnt service.c
>                support/win32 ApacheMonitor.c
>   Log:
>   WIN64: fix some windows specific 64bit warnings
> 
>   -    if (!WriteConsole(hConErr, msg, strlen(msg), &result, NULL) || !result)
>   +    if (!WriteConsole(hConErr, msg, (int)strlen(msg), &result, NULL) || !result)
>   -        if (!WriteConsole(hConErr, count, strlen(count), &result, NULL) 
>   +        if (!WriteConsole(hConErr, count, (int)strlen(count), &result, NULL) 
>   -            lstrcpyn(szBuf, szImagePath, sPos - szImagePath);
>   +            lstrcpyn(szBuf, szImagePath, (int)(sPos - szImagePath));
>   -            TextOut(lpdis->hDC, XBITMAP + 6, y, szBuf, strlen(szBuf)); 
>   +            TextOut(lpdis->hDC, XBITMAP + 6, y, szBuf, (int)strlen(szBuf)); 

Please do not do that any more. I'm sorry but I'm vetoing your patches.
Use the official API, it is well documented.

size_t != int.

Regards,
MT.


Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

Posted by Allan Edwards <ak...@us.ibm.com>.
Mladen Turk wrote:
[snip]
> Both apr and libhttpd has more then 100 of those
> when /Wp64 is used, so IMO we would just hide the problems under
> carpet if just casting every 64->32 warning found.

Agreed, hiding underlying problems is not acceptable.
I am trying to address the many warnings that a
Win64 build spits out in the correct way, not just
for the sake of suppressing warnings.

> For example inside http_protocol.c you have:
(i.e. in the current http_protocol.c code)

> 'int len = strlen(method)', that is wrong by all means, but I
> wouldn't write that as 'int len = (int)strlen(method)' just to make
> the compiler happy, but rather use 'size_t len = strlen(method)'.

Agreed. In the patch just posted you'll see mod_win32.c takes the
approach you suggest, as does a patch I have been working on that
addresses http_protocol.c and other warnings. I will be posting
this shortly.

> Well, that one is probably OK. I was talking about the concept
> of not casting just for the sake of making compiler happy.

Agree.

Thanks, Allan

Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

Posted by Mladen Turk <mt...@apache.org>.
Allan Edwards wrote:
> Mladen Turk wrote:
> 
>> That's true, but the strlen can never be int (negative).
>> The API is DWORD meaning 32 bit unsigned integer, so either
>> cast to API or no cast.
> 
> 
> You are correct that for WriteConsole the cast should
> have been DWORD - I will fix that, thanks
>

Look, I'm sorry if you found my initial post offending cause
I've used the 'v' word, but the way you've headed would make
almost any CRT lib call casted, and that just doesn't make
sense to me. Both apr and libhttpd has more then 100 of those
when /Wp64 is used, so IMO we would just hide the problems under
carpet if just casting every 64->32 warning found.
For example inside http_protocol.c you have:
'int len = strlen(method)', that is wrong by all means, but I
wouldn't write that as 'int len = (int)strlen(method)' just to make
the compiler happy, but rather use 'size_t len = strlen(method)'.

 > For TextOut and lstrcpyn the parameter is in fact int
 > so we either have to cast to int and assmume the
 > string length will never by > 2Gig (seems reasonable),
 > or we need to explicitly check for strlen > 2Gig and
 > fail if it is, or use a different API (if there is one).
 >

Well, that one is probably OK. I was talking about the concept
of not casting just for the sake of making compiler happy.


Regards,
Mladen.

Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

Posted by Allan Edwards <ak...@us.ibm.com>.
Mladen Turk wrote:
> That's true, but the strlen can never be int (negative).
> The API is DWORD meaning 32 bit unsigned integer, so either
> cast to API or no cast.

You are correct that for WriteConsole the cast should
have been DWORD - I will fix that, thanks

For TextOut and lstrcpyn the parameter is in fact int
so we either have to cast to int and assmume the
string length will never by > 2Gig (seems reasonable),
or we need to explicitly check for strlen > 2Gig and
fail if it is, or use a different API (if there is one).

What do you suggest?
Allan

Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, 06 Oct 2004 16:15:51 +0200, Mladen Turk <mt...@apache.org> wrote:
> Jeff Trawick wrote:
> >>
> >>Please do not do that any more. I'm sorry but I'm vetoing your patches.
> >>Use the official API, it is well documented.
> >
> >
> > strlen() returns size_t, TextOut() requires int; somewhere a cast is
> > required, no?  what is a better solution?
> >
> >
> 
> That's true, but the strlen can never be int (negative).
> The API is DWORD meaning 32 bit unsigned integer, so either
> cast to API or no cast.

I'm confused.  The only TextOut() doc I can find says "int", not
"DWORD."  Can you provide a link?  Here is where I looked:

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/gdi/fontext_5yd0.asp

Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

Posted by Mladen Turk <mt...@apache.org>.
Jeff Trawick wrote:
>>
>>Please do not do that any more. I'm sorry but I'm vetoing your patches.
>>Use the official API, it is well documented.
> 
> 
> strlen() returns size_t, TextOut() requires int; somewhere a cast is
> required, no?  what is a better solution?
> 
> 

That's true, but the strlen can never be int (negative).
The API is DWORD meaning 32 bit unsigned integer, so either
cast to API or no cast.

MT.

Re: cvs commit: httpd-2.0/support/win32 ApacheMonitor.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, 06 Oct 2004 15:52:43 +0200, Mladen Turk <mt...@apache.org> wrote:
> 
> 
> ake@apache.org wrote:
> > ake         2004/10/06 06:33:29
> >
> >   Modified:    modules/arch/win32 mod_win32.c
> >                server/mpm/winnt service.c
> >                support/win32 ApacheMonitor.c
> >   Log:
> >   WIN64: fix some windows specific 64bit warnings
> >
> >   -    if (!WriteConsole(hConErr, msg, strlen(msg), &result, NULL) || !result)
> >   +    if (!WriteConsole(hConErr, msg, (int)strlen(msg), &result, NULL) || !result)
> >   -        if (!WriteConsole(hConErr, count, strlen(count), &result, NULL)
> >   +        if (!WriteConsole(hConErr, count, (int)strlen(count), &result, NULL)
> >   -            lstrcpyn(szBuf, szImagePath, sPos - szImagePath);
> >   +            lstrcpyn(szBuf, szImagePath, (int)(sPos - szImagePath));
> >   -            TextOut(lpdis->hDC, XBITMAP + 6, y, szBuf, strlen(szBuf));
> >   +            TextOut(lpdis->hDC, XBITMAP + 6, y, szBuf, (int)strlen(szBuf));
> 
> Please do not do that any more. I'm sorry but I'm vetoing your patches.
> Use the official API, it is well documented.

strlen() returns size_t, TextOut() requires int; somewhere a cast is
required, no?  what is a better solution?