On 2017-07-12 21:21:07 -0700, Brendan Cully wrote: > changeset: 7104:742c96078159 > user: Brendan Cully <bren...@kublai.com> > date: Wed Jul 12 21:20:24 2017 -0700 > link: http://dev.mutt.org/hg/mutt/rev/742c96078159 > > bcache: cast to avoid implicit signed/unsigned comparison in bcache_path > > diffs (12 lines): > > diff -r 51d22025190a -r 742c96078159 bcache.c > --- a/bcache.c Wed Jul 12 12:39:28 2017 -0700 > +++ b/bcache.c Wed Jul 12 21:20:24 2017 -0700 > @@ -71,7 +71,7 @@ > > dprint (3, (debugfile, "bcache_path: rc: %d, path: '%s'\n", len, dst)); > > - if (len < 0 || len >= dstlen-1) > + if (len < 0 || (size_t)len >= dstlen-1) > return -1;
I don't see the reason for this change. The code was correct since len is guaranteed to be non-negative when the test len >= dstlen-1 is performed, so that the comparison without the cast is valid. Casts like that may hide bugs. For instance, if the len < 0 test were missing: if (len >= dstlen-1) would be incorrect (since len may be silently converted to an unsigned type, depending on the platform), but the compiler could warn due to the fact that len can be negative and dstlen-1 is unsigned. And if ((size_t)len >= dstlen-1) would *also* be incorrect since converting negative values to unsigned with a cast makes no sense here. And the compiler would have no way to warn about this issue. Thus the bug would remain undetected. -- Vincent Lefèvre <vinc...@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
