On 31-1-2016 00:13, Willem Jan Withagen wrote:
> On 30-1-2016 23:57, Matt Benjamin wrote:
>> Should we use std::min here (that might require a cast, iirc)?
>
> Well the most important issue I have:
> while(len>0)
> where len is of type size_t has only exactly one chance to be true, aka
> len = 0. negative numbers do not exist.
>
> So casting it to int is a bad(tm) thing.
>
> But as I haven't designed the code, i can only react to the compiler
> error and analyze it. And then my conclusion is that the cast can only
> increase the chance on an error. And thus the compiler is correct in
> triggering.
Sorry,
I did not answer your question.
If using std::min requires a cast, then it will fall in the same pitfall
as the current code does. if there is a std::min version that does not
autocast/promote size_t to int it might work as well.
Using MIN does "the right thing", without the cast.
diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 1bad6a2..13555c9 100644
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -728,7 +728,7 @@ int BlueFS::_read(
left = buf->get_buf_remaining(off);
dout(20) << __func__ << " left " << left << " len " << len << dendl;
- int r = MIN((int)len, left);
+ int r = MIN(len, left);
if (outbl) {
bufferlist t;
t.substr_of(buf->bl, off - buf->bl_off, r);
This does the job on Centos 7 for me....
--WjW
>> ----- Original Message -----
>>> From: "Willem Jan Withagen" <wjw@xxxxxxxxxxx>
>>> To: ceph-devel@xxxxxxxxxxxxxxx
>>> Sent: Saturday, January 30, 2016 8:22:07 AM
>>> Subject: compiling stops at od compare
>>>
>>> When trying to compile on CentOS 7, gcc = 4.8.3
>>>
>>> os/bluestore/BlueFS.cc: In member function ‘int
>>> BlueFS::_read(BlueFS::FileReader*, BlueFS::FileReaderBuffer*, uint64_t,
>>> size_t, ceph::bufferlist*, char*)’:
>>> os/bluestore/BlueFS.cc:731:31: warning: comparison between signed and
>>> unsigned integer expressions [-Wsign-compare]
>>> int r = MIN((int)len, left);
>>>
>>> This MIN is used to determine the amount of buffer that is still left
>>> to be filed.
>>> And here len and left are both size_t..., suggesting that both cannot be
>>> negative. So either both need to be promoted/cast, or neither.
>>>
>>> The cast (int)len suggests that len could be negative.
>>> The part where that could happen is at line 750:
>>>
>>> off += r;
>>> len -= r;
>>> ret += r;
>>>
>>> So there the loop exit needs len to be exactly equal to r. Even if the
>>> loop specifies while(len>0). if len gets "negative" it grows into
>>> something rather big.
>>>
>>> Now if len never gets negative then it also does not need to get cast to
>>> int. If it does, then in the unsigned case it will always be larger than
>>> left.
>>>
>>> So bottomline is that the cast serves no purpose?
>>> Removing it fixes compilation.
>>>
>>> --WjW
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
