Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: xsrc/external/mit/fontconfig/dist/src



On Thu, Jun 27, 2013 at 12:44:11 +0000, Thomas Klausner wrote:

> Modified Files:
>       xsrc/external/mit/fontconfig/dist/src: fcname.c
> 
> Log Message:
> Fix a comparison of constant warning with clang
> 
> >From upstream:
> commit 9acc14c34a372b54f9075ec3611588298fb2a501
> Author: Akira TAGOH <akira%tagoh.org@localhost>
> Date:   Wed Jun 26 12:03:38 2013 +0900

Their fix is incorrect.

-           if (t->type == (unsigned int) -1 || type == t->type)
+           if ((unsigned int) t->type == (unsigned int) -1 || type == t->type)

it does wrong thing with short enums, and non-eabi arm ports still use
it, as far as I understand.

The enum in question contains only small values, so with -fshort-enums
(default for older arms ABIs) gcc will use the narrowest integral type
that can hold all the values, in this case single byte is enough, and
(though I don't remember chapter and verse) it's likely to be
unsigned.  Thus

  t->type = -1;

will be converted to uint8_t and stored as 0xff, and when cast to
unsigned int it will be 0xff, so it will NOT compare equal with
(unsigned int)-1

Consider something like:

#include <stdio.h>

enum e { E0, E1 };
volatile enum e e;

int
main(void)
{
    printf("sizeof enum = %zu\n", sizeof(e));

    e = -1;
    if ((unsigned int)e == (unsigned int)-1) {
        printf("equal\n");
    }
    else {
        printf("NOT equal\n");
    }
    return 0;
}


$ cc -Wall -Wextra enum.c && ./a.out 
sizeof enum = 4
equal

$ cc -Wall -Wextra -fshort-enums enum.c && ./a.out 
enum.c: In function 'main':
enum.c:12:5: warning: comparison is always false due to limited range of data 
type
sizeof enum = 1
NOT equal

-1 should really be in the enum as one of the enumeration constants.

-uwe


Home | Main Index | Thread Index | Old Index