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. --WjW > > Matt > > ----- 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
