Cyrille Artho wrote:
> Scott Kostyshak wrote:
>> On Sun, May 18, 2014 at 7:58 PM, Richard Heck <rgh...@lyx.org> wrote:
>>> On 05/18/2014 07:22 PM, Scott Kostyshak wrote:
>>>>
>>>> Is this OK? Note that converting the left hand side to an unsigned int
>>>> would work also, because it was checked just above that nRead is >=0.
>>>> But I figure converting the signed to unsigned is better in general as
>>>> long as the numbers are not expected to be large.
Please be careful with "never expected to be large". The warning you are
adressing is about a subtle error which usually does not occur, but there is
a small chance that it can occur. When fixing warnings like that I very much
prefer to fix them in a 100% safe way without any assumptions. Otherwise one
could just switch the appropriate warning off.
> I would change variable "nRead" to ssize_t, as it's common that -1 is
> used as a return value in case of a read error, and signed/unsignedness
> conversions produce potentially dangerous patterns. (Not only the case
> where code is moved around, but also if it's copy/pasted.)
>
> You are right that unsigned int may overflow in the far future - hence
> use ssize_t, which is the right data type here (at least on Mac OS X,
> I'm out of office now so I can't check other platforms).
It is the correct type on linux as well. Therefore, the approach suggested
by Cyrille is IMHO the best one: Change the type of nRead to ssize_t (note
the two s), and change the if-condition to
if (static_cast<size_t>(nRead) < buf.size() - 1) {
This is 100% safe: We know that nRead > 0 at the time the cast occurs, and
ssize_t has the same number of bits as size_t, and buf.size() returns
size_t, and buf.size() is never 0. The static_cast is better than a C-style
cast, since you can search for it, and it expresses what is wanted (C-style
casts could also be used to cast constness away etc).
Georg
