> > > > > Without having studied it closely, it might also > highlight a bug > > > > on > > > > > failure of the second clause -- if the _setmode > fails, shouldn't > > > > > _close be called instead of CloseHandle, and -1 returned? > > > > > (CloseHandle would still be called on failure of the > > > > _open_osfhandle, > > > > > obviously) > > > > > > > > I agree that this code is both wrong and unreadable > (although in > > > > practice the _setmode will probably never fail, which > is why our > > > > attention hasn't been drawn to it). Is someone going > to submit a > > > > patch? I'm hesitant to change the code myself since > I'm not in a > > > > position to test it. > > > > > > I can look at fixing that. Is it something we want to do > for 8.2, or > > > wait until 8.3? If there's a hidden bug, I guess 8.2? > > > > Magnus, is this the right fix? > > Claudio sent me some small updates to the patch; updated > version attached.
If we're going for maximum readability, I'd actually split + else if (fileFlags & (O_TEXT | O_BINARY) && + _setmode(fd, fileFlags & (O_TEXT | O_BINARY)) < 0) into two different if statements as wlel. Ee.g. else if (fileFlags (O_TEXT | O_BINARY)) { if (_setmode() < 0) { failure(); } } Haven't tested the patch yet, but it looks ok. //Magnus ---------------------------(end of broadcast)--------------------------- TIP 6: explain analyze is your friend