"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
make close(2) error checks consistent
To:
gameoftrees@openbsd.org
Date:
Thu, 21 Jan 2021 20:12:08 +0100

Download raw body.

Thread
make close(2) failure checks consistent; check 'close() == -1' everywhere

ok?
 
diff 62c69b89dbed5ae63ec9b0fc725898fde8b914d0 57c0188566d9565680fa0584ac6c9f3e4dfb0bc7
blob - 3986b23c81fbc180d2bf37f92161e9fd2855949c
blob + b26da2a0b727f30ffdba6e4970dafa32b3856f53
--- lib/buf.c
+++ lib/buf.c
@@ -303,7 +303,7 @@ buf_write(BUF *b, const char *path, mode_t mode)
 	if (fchmod(fd, mode) < 0)
 		err = got_error_from_errno2("fchmod", path);
 
-	if (close(fd) != 0 && err == NULL)
+	if (close(fd) == -1 && err == NULL)
 		err = got_error_from_errno2("close", path);
 
 	return err;
@@ -328,7 +328,7 @@ buf_write_stmp(BUF *b, char *template)
 		(void)unlink(template);
 	}
 
-	if (close(fd) != 0 && err == NULL)
+	if (close(fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 
 	return err;
blob - 134a933494056e53673c29dc390c0621d0fdb1c2
blob + bc5756e623d51979c01828389337089965f782a8
--- lib/fetch.c
+++ lib/fetch.c
@@ -557,7 +557,7 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
 		    GOT_PATH_PROG_FETCH_PACK, tmppackpath);
 	}
 
-	if (close(imsg_fetchfds[1]) != 0) {
+	if (close(imsg_fetchfds[1]) == -1) {
 		err = got_error_from_errno("close");
 		goto done;
 	}
@@ -732,7 +732,7 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
 	} else if (idxpid == 0)
 		got_privsep_exec_child(imsg_idxfds,
 		    GOT_PATH_PROG_INDEX_PACK, tmppackpath);
-	if (close(imsg_idxfds[1]) != 0) {
+	if (close(imsg_idxfds[1]) == -1) {
 		err = got_error_from_errno("close");
 		goto done;
 	}
blob - bc4cf864a87ae6ca01a4fa56e150cef5110d17aa
blob + 71a5d735012116d324357ccca04f1c7be867e6c2
--- lib/lockfile.c
+++ lib/lockfile.c
@@ -82,7 +82,7 @@ got_lockfile_unlock(struct got_lockfile *lf)
 
 	if (lf->path && lf->fd != -1 && unlink(lf->path) != 0)
 		err = got_error_from_errno("unlink");
-	if (lf->fd != -1 && close(lf->fd) != 0 && err == NULL)
+	if (lf->fd != -1 && close(lf->fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	free(lf->path);
 	free(lf->locked_path);
blob - 7bd55922b5b83ac06fbadac4c69364802b1a6179
blob + 77815dadd4302423c4d226f5f48df31b08dee415
--- lib/object.c
+++ lib/object.c
@@ -237,7 +237,7 @@ start_pack_privsep_child(struct got_pack *pack, struct
 		/* not reached */
 	}
 
-	if (close(imsg_fds[1]) != 0)
+	if (close(imsg_fds[1]) == -1)
 		return got_error_from_errno("close");
 	pack->privsep_child->imsg_fd = imsg_fds[0];
 	pack->privsep_child->pid = pid;
@@ -362,7 +362,7 @@ read_object_header_privsep(struct got_object **obj, st
 		/* not reached */
 	}
 
-	if (close(imsg_fds[1]) != 0) {
+	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
 		free(ibuf);
 		return err;
@@ -542,7 +542,7 @@ read_commit_privsep(struct got_commit_object **commit,
 		/* not reached */
 	}
 
-	if (close(imsg_fds[1]) != 0) {
+	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
 		free(ibuf);
 		return err;
@@ -731,7 +731,7 @@ read_tree_privsep(struct got_tree_object **tree, int o
 		/* not reached */
 	}
 
-	if (close(imsg_fds[1]) != 0) {
+	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
 		free(ibuf);
 		return err;
@@ -1087,7 +1087,7 @@ read_blob_privsep(uint8_t **outbuf, size_t *size, size
 		/* not reached */
 	}
 
-	if (close(imsg_fds[1]) != 0) {
+	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
 		free(ibuf);
 		return err;
@@ -1163,7 +1163,7 @@ open_blob(struct got_blob_object **blob, struct got_re
 	}
 
 	if (outbuf) {
-		if (close(outfd) != 0 && err == NULL)
+		if (close(outfd) == -1 && err == NULL)
 			err = got_error_from_errno("close");
 		outfd = -1;
 		(*blob)->f = fmemopen(outbuf, size, "rb");
@@ -1444,7 +1444,7 @@ read_tag_privsep(struct got_tag_object **tag, int obj_
 		/* not reached */
 	}
 
-	if (close(imsg_fds[1]) != 0) {
+	if (close(imsg_fds[1]) == -1) {
 		err = got_error_from_errno("close");
 		free(ibuf);
 		return err;
blob - 71e76e0ebe9a73692b22f11470e926c20f44b98c
blob + 0d4c453c0dcbf0a26403587fe25789e2a5bc7dd5
--- lib/object_create.c
+++ lib/object_create.c
@@ -203,7 +203,7 @@ got_object_blob_file_create(struct got_object_id **id,
 	rewind(*blobfile);
 done:
 	free(header);
-	if (fd != -1 && close(fd) != 0 && err == NULL)
+	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (err) {
 		free(*id);
blob - b46583a6e3ce2fe952e1c30726026d492c7f5734
blob + 712b0f55c1bb511c3d3829528964d5036a503d9d
--- lib/pack.c
+++ lib/pack.c
@@ -408,7 +408,7 @@ got_packidx_close(struct got_packidx *packidx)
 		free(packidx->hdr.large_offsets);
 		free(packidx->hdr.trailer);
 	}
-	if (close(packidx->fd) != 0 && err == NULL)
+	if (close(packidx->fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	free(packidx);
 
@@ -525,7 +525,7 @@ got_pack_stop_privsep_child(struct got_pack *pack)
 	if (err)
 		return err;
 	err = got_privsep_wait_for_child(pack->privsep_child->pid);
-	if (close(pack->privsep_child->imsg_fd) != 0 && err == NULL)
+	if (close(pack->privsep_child->imsg_fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	free(pack->privsep_child);
 	pack->privsep_child = NULL;
@@ -540,7 +540,7 @@ got_pack_close(struct got_pack *pack)
 	err = got_pack_stop_privsep_child(pack);
 	if (pack->map && munmap(pack->map, pack->filesize) == -1 && !err)
 		err = got_error_from_errno("munmap");
-	if (pack->fd != -1 && close(pack->fd) != 0 && err == NULL)
+	if (pack->fd != -1 && close(pack->fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	pack->fd = -1;
 	free(pack->path_packfile);
blob - 4fb6f24032dafb92991f1671b8252d6b045fa81b
blob + 2a8fcc8c9e2afd1ce22356056ac095e73d2c28c3
--- lib/privsep.c
+++ lib/privsep.c
@@ -2307,7 +2307,7 @@ got_privsep_unveil_exec_helpers(void)
 void
 got_privsep_exec_child(int imsg_fds[2], const char *path, const char *repo_path)
 {
-	if (close(imsg_fds[0]) != 0) {
+	if (close(imsg_fds[0]) == -1) {
 		fprintf(stderr, "%s: %s\n", getprogname(), strerror(errno));
 		_exit(1);
 	}
blob - 4a6e13fd0afc2e6e9bb7a10467c39500a78a8734
blob + 1118dcefd897d4540eae1a9353d4bec5937e454d
--- lib/repository.c
+++ lib/repository.c
@@ -740,7 +740,7 @@ got_repo_close(struct got_repository *repo)
 		    repo->privsep_children[i].pid);
 		if (child_err && err == NULL)
 			err = child_err;
-		if (close(repo->privsep_children[i].imsg_fd) != 0 &&
+		if (close(repo->privsep_children[i].imsg_fd) == -1 &&
 		    err == NULL)
 			err = got_error_from_errno("close");
 	}
blob - 9eb2d798442973db6cdaca5e079d37834bf0ca46
blob + cc6f6a916c25407b06f404601ca012ed38d5a6fc
--- lib/worktree.c
+++ lib/worktree.c
@@ -504,7 +504,7 @@ got_worktree_close(struct got_worktree *worktree)
 	free(worktree->base_commit_id);
 	free(worktree->head_ref_name);
 	if (worktree->lockfd != -1) {
-		if (close(worktree->lockfd) != 0)
+		if (close(worktree->lockfd) == -1)
 			err = got_error_from_errno2("close",
 			    got_worktree_get_root_path(worktree));
 	}
@@ -914,7 +914,7 @@ done:
 	if (symlinkf && fclose(symlinkf) == EOF && err == NULL)
 		err = got_error_from_errno2("fclose", symlink_path);
 	free(symlink_path);
-	if (merged_fd != -1 && close(merged_fd) != 0 && err == NULL)
+	if (merged_fd != -1 && close(merged_fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (f_orig && fclose(f_orig) != 0 && err == NULL)
 		err = got_error_from_errno("fclose");
@@ -1581,7 +1581,7 @@ install_blob(struct got_worktree *worktree, const char
 	}
 
 done:
-	if (fd != -1 && close(fd) != 0 && err == NULL)
+	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (tmppath != NULL && unlink(tmppath) == -1 && err == NULL)
 		err = got_error_from_errno2("unlink", tmppath);
@@ -3623,7 +3623,7 @@ worktree_status(struct got_worktree *worktree, const c
 	}
 done:
 	free_ignores(&arg.ignores);
-	if (fd != -1 && close(fd) != 0 && err == NULL)
+	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	free(ondisk_path);
 	return err;
blob - 7e406576e1b1c9b3b3f5259a819620d83680b7f7
blob + b301e317864022d4ee7c45bf4c15472ef3d2554e
--- libexec/got-read-blob/got-read-blob.c
+++ libexec/got-read-blob/got-read-blob.c
@@ -167,11 +167,11 @@ done:
 			if (fclose(f) != 0 && err == NULL)
 				err = got_error_from_errno("fclose");
 		} else if (imsg.fd != -1) {
-			if (close(imsg.fd) != 0 && err == NULL)
+			if (close(imsg.fd) == -1 && err == NULL)
 				err = got_error_from_errno("close");
 		}
 		if (imsg_outfd.fd != -1) {
-			if (close(imsg_outfd.fd) != 0 && err == NULL)
+			if (close(imsg_outfd.fd) == -1 && err == NULL)
 				err = got_error_from_errno("close");
 		}
 
@@ -190,7 +190,7 @@ done:
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - 02685f91e98ddcfe9403eec99e73b35f964a30fb
blob + 445974d47a5ce8a43cc4495c78aeec7bf2b66cb7
--- libexec/got-read-commit/got-read-commit.c
+++ libexec/got-read-commit/got-read-commit.c
@@ -151,7 +151,7 @@ done:
 			if (fclose(f) != 0 && err == NULL)
 				err = got_error_from_errno("fclose");
 		} else if (imsg.fd != -1) {
-			if (close(imsg.fd) != 0 && err == NULL)
+			if (close(imsg.fd) == -1 && err == NULL)
 				err = got_error_from_errno("close");
 		}
 		imsg_free(&imsg);
@@ -166,7 +166,7 @@ done:
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - b0457c7fd3f8239ee5f52338976899dd8ccaefee
blob + eadef712009b2da9a414361c8395ccbd43f2b1aa
--- libexec/got-read-gitconfig/got-read-gitconfig.c
+++ libexec/got-read-gitconfig/got-read-gitconfig.c
@@ -406,7 +406,7 @@ main(int argc, char *argv[])
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - 2fe537dc6e1f300194d9d5d74a0b2e170c9a0010
blob + 682d443c685d81724cfda1569e2030e2bb02b87c
--- libexec/got-read-gotconfig/got-read-gotconfig.c
+++ libexec/got-read-gotconfig/got-read-gotconfig.c
@@ -392,7 +392,7 @@ main(int argc, char *argv[])
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - 5e190138a78d67199c00504c2c04e47659ff322c
blob + 96f238defd60d0d973d36d4b4e6c4a60845cefa6
--- libexec/got-read-object/got-read-object.c
+++ libexec/got-read-object/got-read-object.c
@@ -111,7 +111,7 @@ main(int argc, char *argv[])
 
 		err = got_privsep_send_obj(&ibuf, obj);
 done:
-		if (close(imsg.fd) != 0 && err == NULL)
+		if (close(imsg.fd) == -1 && err == NULL)
 			err = got_error_from_errno("close");
 		imsg_free(&imsg);
 		if (obj)
@@ -127,7 +127,7 @@ done:
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - 27d10ab755d53e96136938c0a9d1d5a574974686
blob + dde61e8f675ee7fe4f7568b446f9ae171f42c7f5
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -1009,7 +1009,7 @@ main(int argc, char *argv[])
 			break;
 		}
 
-		if (imsg.fd != -1 && close(imsg.fd) != 0 && err == NULL)
+		if (imsg.fd != -1 && close(imsg.fd) == -1 && err == NULL)
 			err = got_error_from_errno("close");
 		imsg_free(&imsg);
 		if (err)
@@ -1028,7 +1028,7 @@ main(int argc, char *argv[])
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - aad8331a9697582ede82b7f52785bf4c8b173cb8
blob + df8f85a5d4d7a2ff3f490a02166a0717327abf72
--- libexec/got-read-tag/got-read-tag.c
+++ libexec/got-read-tag/got-read-tag.c
@@ -143,7 +143,7 @@ done:
 			if (fclose(f) != 0 && err == NULL)
 				err = got_error_from_errno("fclose");
 		} else if (imsg.fd != -1) {
-			if (close(imsg.fd) != 0 && err == NULL)
+			if (close(imsg.fd) == -1 && err == NULL)
 				err = got_error_from_errno("close");
 		}
 		imsg_free(&imsg);
@@ -158,7 +158,7 @@ done:
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }
blob - 741d51f94fe1970f533774c6866a6f49965e182f
blob + 583cfe5cbae83c86ba09ce1d94a23539ebbc8af8
--- libexec/got-read-tree/got-read-tree.c
+++ libexec/got-read-tree/got-read-tree.c
@@ -149,7 +149,7 @@ done:
 			if (fclose(f) != 0 && err == NULL)
 				err = got_error_from_errno("fclose");
 		} else if (imsg.fd != -1) {
-			if (close(imsg.fd) != 0 && err == NULL)
+			if (close(imsg.fd) == -1 && err == NULL)
 				err = got_error_from_errno("close");
 		}
 		imsg_free(&imsg);
@@ -164,7 +164,7 @@ done:
 			got_privsep_send_error(&ibuf, err);
 		}
 	}
-	if (close(GOT_IMSG_FD_CHILD) != 0 && err == NULL)
+	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err ? 1 : 0;
 }