[PATCH:libXi] Fix size comparison.

[Thomas Klausner]

On Thu, Jun 27, 2013 at 07:11:44AM -0700, Alan Coopersmith wrote:
> On 06/27/13 05:55 AM, Thomas Klausner wrote:
> >This addresses the following clang-3.4 warning:
> >XGetFCtl.c:129:32: warning: comparison of constant 268435455 with expression of type 'CARD16' (aka 'unsigned short') is always false
> >       [-Wtautological-constant-out-of-range-compare]
> >                 if (strf->num_syms_supported >= (INT_MAX / sizeof(KeySym)))
> >                     ~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~
> >1 warning generated.
> >---
> >  src/XGetFCtl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/src/XGetFCtl.c b/src/XGetFCtl.c
> >index bb50bf3..100ae9e 100644
> >--- a/src/XGetFCtl.c
> >+++ b/src/XGetFCtl.c
> >@@ -126,7 +126,7 @@ XGetFeedbackControl(
> >  	    {
> >  		xStringFeedbackState *strf = (xStringFeedbackState *) f;
> >
> >-		if (strf->num_syms_supported >= (INT_MAX / sizeof(KeySym)))
> >+		if (strf->num_syms_supported >= (SHRT_MAX / sizeof(KeySym)))
> >  		    goto out;
> >  		size += sizeof(XStringFeedbackState) +
> >  		    (strf->num_syms_supported * sizeof(KeySym));
> >
> 
> Don't lower the limit - it was added to make sure the calculation of:
> 	(strf->num_syms_supported * sizeof(KeySym))
> doesn't overflow the integer calculations it's going to be added to, but
> apparently my cross referencing of which fields were CARD32's we needed
> to check vs. which were CARD16's that can't overflow got that case wrong
> and treated it as if it were CARD32.
> 
> Changing INT_MAX to SHRT_MAX introduces new bugs by making the libary fail
> to handle valid responses from the X server.

Ok, thanks for the explanation. I'll send a patch removing the check
instead.

> If you simply can't bear to have the clang warning, the check should be
> deleted, not broken.

I'm not looking at warnings for the fun of it.

Many X sources are imported into the NetBSD source tree in the xsrc
repository. By default, they are built using gcc, but we are in the
process of migrating to clang (like FreeBSD did). Most of the warnings
for which I have sent patches currently break the build of xsrc using
clang using the default flags enabled in NetBSD's infrastructure.

I have the choice of adding compiler flags to the Makefile to ignore
those warnings or looking at the warnings and trying to fix them. I
opted for the latter. If you prefer me to do the former, just let me
know.
 Thomas