[PATCH:libXi] Fix size comparison.

Alan Coopersmith alan.coopersmith at oracle.com
Thu Jun 27 07:11:44 PDT 2013


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.

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

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc


More information about the xorg-devel mailing list