X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f
Date: Thu, 12 May 2005 22:17:34 -0400
Message-Id: <200505130217.j4D2HY1G005382@envy.delorie.com>
From: DJ Delorie <dj AT delorie DOT com>
To: djgpp-workers AT delorie DOT com
In-reply-to: <200505130411.14826.juan.guerrero@gmx.de> (message from Juan
	Manuel Guerrero on Fri, 13 May 2005 04:11:14 +0200)
Subject: Re: More signedess issues in the cvs tree
References:  <200505130411 DOT 14826 DOT juan DOT guerrero AT gmx DOT de>
Reply-To: djgpp-workers AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk


If this is a new warning in gcc 4, perhaps we can disable it for now
until we bring the sources into compliance.  I think we've done that
before.  But that doesn't mean we should ignore these warnings, and we
should plan on enabling it in the future if possible.

> May be it will be worth to thincking about making the signedess
> warning not to abort compilation instead of correcting almost every
> source file in the cvs tree.

No, because it's not the majority of the obvious cases we care about
that way.  It's the rare subtle bug that these are for, and they
really do make a difference.  Besides...

> -    i = __dpmi_set_page_attributes(&meminfo, pageprot);
> +    i = __dpmi_set_page_attributes(&meminfo, (short *)pageprot);

The bug here is that __dpmi_set_page_attributes should take an
unsigned short *, not a signed short *.  The value is a bitfield, not
an integer.  Technically, it could take either a void* or a struct*
too.  But it certainly shouldn't require you to cast from an
appropriate type to an obviously inappropriate type.  If it weren't
for backwards compatibility, this would be an easy fix, but we should
consider both correctness and compatibility here.

> -      sprintf(mnt_fsname, "%c:\\DBLSPACE.%03u", 'A' + host, r.h.bh);
> +      sprintf((char *)mnt_fsname, "%c:\\DBLSPACE.%03u", 'A' + host, r.h.bh);

In this case, mnt_fsname should probably be a char*.

Etc.

Each place where the warning comes up, we need to investigate, and
determine what the *right* way to fix it is.  That's why we enable
these warnings in the first place.  We do *not* want to blindly
bulk-fix these types of things, because that's where bugs creep in.