On Tue, Feb 19, 2019 at 05:23:38PM -0600, Eric Sandeen wrote:
> openfile() stats a path to determine whether it is a pipe, and then
> opens it with flags based on the type it saw during the stat. It's
> possible that the file at that path changes in between, and Coverity
> points this out.
>
> Instead, always open O_NONBLOCK, stat the fd we got, and turn the
> flag back off via fcntl() if it's not a pipe.
>
> Addresses-Coverity-ID: 1442788 ("Time of check time of use")
For O_NONBLOCK I think there is zero harm in the code as it stands,
but I don't really care about it tht much.
I do wonder what happens if need to conditionally use O_NONBLOCK on
open(), though....
> @@ -112,6 +106,22 @@ openfile(
> }
> }
>
> + if (fstat(fd, &st) < 0) {
> + perror("stat");
> + close(fd);
> + return -1;
> + }
> +
> + /* We only want to keep nonblocking behavior for pipes */
> + if (!S_ISFIFO(st.st_mode)) {
> + oflags &= ~O_NONBLOCK;
> + if (fcntl(fd, F_SETFL, oflags) < 0) {
Don't you need to do oflags = fcntl(fd, F_GETFL, NULL) first?
> + perror("fcntl");
> + close(fd);
> + return -1;
> + }
can you add a "goto out_close;" error jump to this function now that
this is the fourth and fifth places that have this same close/return
on error...
CHeers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
