Closed Bug 1085151 Opened 7 years ago Closed 7 years ago
char signed vs unsigned build failures
seamonkey fails to build on powerpc with the following type of errors comm-central/mailnews/base/search/src/nsMsgFilterList.cpp:493:22: error: comparison is always true due to limited range of data type [-Werror=type-limits] while (curChar != -1); 'char' on powerpc is unsigned.
Comment on attachment 8507548 [details] [diff] [review] signed_char.diff Don't you also need to change the return value of ReadChar() as that is the one that may return -1 ? Or maybe we should convert all to unsigned char and have 255 be the special value.
Updated version of the patch that changes the return value of ReadChar() and catches a few other char usages in the file.
What would you think of the approach? Does this work with powerpc? Because nsIInputStream.read is expecting a charPtr (which is char *) I hate to switch to signed or unsigned char to make a comparison compile. I would rather just do a proper comparison. Magic numbers like -1 are generally discouraged, so in the process I cleaned that up with a macro.
8511590 builds for me on powerpc
So are you satisfied with the revised patch? This is really my suggested changes to your patch, so you need to also be OK with it.
I am satisfied with the revised patch( 8511590)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 36.0
This is the wrong approach. 0xFF is a valid character (ÿ in ISO-8869-1, for example), and any attempt to use a character as an EOF marker is doomed to fail.
(In reply to Andreas Schwab from comment #11) > Created attachment 8544581 [details] [diff] [review] > Use out-of-bounds value as EOF marker. > > This is the wrong approach. 0xFF is a valid character (ÿ in ISO-8869-1, for > example), and any attempt to use a character as an EOF marker is doomed to > fail. This bug was about fixing a build failure, and did not introduce the concept of using 0xFF as an EOF marker. Please feel free to file a new bug where the issue of possible problems with using EOF in this manner can be discussed directly.
Andreas, The patch looks like a good idea to me but you will need to open a new bug for it.
You need to log in before you can comment on or make changes to this bug.