Bug 144439 - [ARM] Don't compare unsigned chars to EOF (-1)
Summary: [ARM] Don't compare unsigned chars to EOF (-1)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
: 104613 (view as bug list)
Depends on:
Blocks: 108645
  Show dependency treegraph
 
Reported: 2015-04-30 02:21 PDT by Csaba Osztrogonác
Modified: 2015-05-11 02:18 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.78 KB, patch)
2015-04-30 02:39 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2015-04-30 02:21:50 PDT
build error no1:
-----------------
../../Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp: In function 'WTF::String WebCore::nextToken(FILE*)':
../../Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp:69:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
         if (ch == EOF || (isASCIISpace(ch) && index)) // Break on non-initial ASCII space.
                ^
cc1plus: all warnings being treated as errors

build error no2:
-----------------
../../Source/WebKit2/Shared/linux/WebMemorySamplerLinux.cpp: In function 'WTF::String WebKit::nextToken(FILE*)':
../../Source/WebKit2/Shared/linux/WebMemorySamplerLinux.cpp:70:16: error: comparison is always false due to limited range of data type [-Werror=type-limits]
         if (ch == EOF || (isASCIISpace(ch) && index)) // Break on non-initial ASCII space.
                ^
cc1plus: all warnings being treated as errors

----

char ch = fgetc(file); ----> fgetc returns an int which is casted to char 
(signed char on X86_64 and unsigned char on ARM). EOF is defined as -1. (int)

If fgetc returns with EOF, it will be 255 after casting unsigned char.
When comparing the unsigned char 255 and the int -1, the unsigned char 
255 will be casted to int 255 which will never be equal to -1.

$ man fgetc
[snip]
    fgetc() reads the next character from stream and returns it as an unsigned char cast to an int, or EOF on end of file or error.
[snip]

The correct fix is to avoid casting the return value of fgetc.
Comment 1 Csaba Osztrogonác 2015-04-30 02:36:56 PDT
Just to document, http://trac.webkit.org/changeset/127195 added the
Source/WebKit2/Shared/linux/WebMemorySamplerLinux.cpp change and
http://trac.webkit.org/changeset/177216 added (copied) this bug
into Source/WebCore/platform/linux/MemoryPressureHandlerLinux.cpp.
Comment 2 Csaba Osztrogonác 2015-04-30 02:39:58 PDT
Created attachment 252048 [details]
Patch
Comment 3 Geoffrey Garen 2015-04-30 10:41:36 PDT
Comment on attachment 252048 [details]
Patch

r=me
Comment 4 Csaba Osztrogonác 2015-05-04 01:43:49 PDT
Comment on attachment 252048 [details]
Patch

Clearing flags on attachment: 252048

Committed r183740: <http://trac.webkit.org/changeset/183740>
Comment 5 Csaba Osztrogonác 2015-05-04 01:43:57 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Csaba Osztrogonác 2015-05-11 02:18:47 PDT
*** Bug 104613 has been marked as a duplicate of this bug. ***