Michael Tokarev <m...@tls.msk.ru> writes: > 15.11.2014 13:06, arei.gong...@huawei.com wrote: >> From: Gonglei <arei.gong...@huawei.com> >> >> In this false branch, fd will leak when it is zero. >> Change the testing condition. > > Why fd==0 is a concern here? It is a very unlikely > situation that fd0 will be picked - firstly because > fd0 is almost always open, and second - even if it > isn't open, it will be picked much earlier than this > code path, ie, some other file will use fd0 before. > > But even if the concern is real (after all, better > stay correct than spread bad code pattern, even if > in reality we don't care as this can't happen), why > not add 0 to the equality? > > Why people especially compare with -1? Any negative > value is illegal here and in lots of other places, > and many software packages used to return -errno in > error cases, which is definitely != -1. I'm not > saying that comparing with -1 is bad in _this_ > particular case, but why not do it generally in > all cases? > > More, comparing with 0 is faster and shorter than > comparing with -1... > > So if it were me, I'd change it to >= 0, not to > == -1. Here and in all other millions of places > in qemu code where it compares with -1... ;)
Yup. In the case of close(), I wouldn't even bother to check, except Coverity gets excited when it sees an invalid fd flow into close(). Rightfully so when the invalid fd is non-negative, overeager when it's negative.
