On 29 Jan 2014, Al Viro outgrape: > On Wed, Jan 29, 2014 at 11:57:20AM +0000, Nick Alcock wrote: >> ssize_t ret = -EBADF; >> >> - if (pos < 0) >> + f = fdget(fd); >> + if ((pos < 0) && (!f.file || !unsigned_offsets(f.file))) { >> + fdput(f); >> return -EINVAL; >> + } > > ... and now pread(-1, ...) fails with EINVAL instead of EBADF. Sorry, I don't see it. If the fh is invalid, control flow is unchanged unless pos is also < 0 (that's an && outside the bracketed section, not an ||, and nothing I've touched changes ret outside that conditional branch): if pos *is* < 0, we'd have had an EINVAL before and we have one now, likewise unchanged. What am I missing? (Or did you miss the brackets enclosing (!f.file || !unsigned_offsets(f.file))? If so, I'm not surprised: it would really be easier to read if that function had the inverse sense, 'signed_offsets()'...) -- NULL && (void) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html