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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
support for non-const basename(3)
To:
gameoftrees@openbsd.org
Date:
Mon, 19 Oct 2020 23:09:22 +0200

Download raw body.

Thread
It looks like naddy is being serious and is going to switch OpenBSD
to non-const POSIX basename(3) any time now.

This patch should keep Got working when this happens.

To keep the conversion simple I chose to use the got_path_basename()
wrapper everywhere, except in the resolve_symlink() function where
strlcpy into a temporary buffer seems simpler and avoids the additional
malloc/free dance.

ok?

diff refs/heads/main refs/heads/basename
blob - 6535a4bdb0b33f77b5e762d754afffbf8ae5d25e
blob + 9ff54863f6b41daa7b9bf2b9cb46bf28b42ffe52
--- got/got.c
+++ got/got.c
@@ -2600,7 +2600,7 @@ cmd_checkout(int argc, char *argv[])
 	const char *path_prefix = "";
 	const char *branch_name = GOT_REF_HEAD;
 	char *commit_id_str = NULL;
-	char *cwd = NULL, *path = NULL;
+	char *cwd = NULL;
 	int ch, same_path_prefix, allow_nonempty = 0;
 	struct got_pathlist_head paths;
 	struct got_checkout_progress_arg cpa;
@@ -2639,6 +2639,7 @@ cmd_checkout(int argc, char *argv[])
 #endif
 	if (argc == 1) {
 		char *base, *dotgit;
+		const char *path;
 		repo_path = realpath(argv[0], NULL);
 		if (repo_path == NULL)
 			return got_error_from_errno2("realpath", argv[0]);
@@ -2648,25 +2649,21 @@ cmd_checkout(int argc, char *argv[])
 			goto done;
 		}
 		if (path_prefix[0])
-			path = strdup(path_prefix);
+			path = path_prefix;
 		else
-			path = strdup(repo_path);
-		if (path == NULL) {
-			error = got_error_from_errno("strdup");
+			path = repo_path;
+		error = got_path_basename(&base, path);
+		if (error)
 			goto done;
-		}
-		base = basename(path);
-		if (base == NULL) {
-			error = got_error_from_errno2("basename", path);
-			goto done;
-		}
 		dotgit = strstr(base, ".git");
 		if (dotgit)
 			*dotgit = '\0';
 		if (asprintf(&worktree_path, "%s/%s", cwd, base) == -1) {
 			error = got_error_from_errno("asprintf");
+			free(base);
 			goto done;
 		}
+		free(base);
 	} else if (argc == 2) {
 		repo_path = realpath(argv[0], NULL);
 		if (repo_path == NULL) {
@@ -2785,7 +2782,6 @@ done:
 	free(repo_path);
 	free(worktree_path);
 	free(cwd);
-	free(path);
 	return error;
 }
 
blob - 0c75579cb470e296c9a97a5ea0ac1e14f342971a
blob + 89384c49f08bc3027b3bab5aea7dd94029669839
--- include/got_path.h
+++ include/got_path.h
@@ -118,7 +118,7 @@ const struct got_error *got_path_dirname(char **, cons
 const struct got_error *got_path_dirent_type(int *, const char *,
     struct dirent *);
 
-/* basename(3) with dynamically allocated result. */
+/* basename(3) with dynamically allocated result and unmodified input. */
 const struct got_error *got_path_basename(char **, const char *);
 
 /* Strip trailing slashes from a path; path will be modified in-place. */
blob - a13a191e043c94ff5f692e1f875fda3b1eb9cb21
blob + a8e56d3ae101b242e7f90d82f2276a5ab517a4fb
--- lib/object.c
+++ lib/object.c
@@ -1855,6 +1855,7 @@ resolve_symlink(char **link_target, const char *path,
     struct got_object_id *commit_id, struct got_repository *repo)
 {
 	const struct got_error *err = NULL;
+	char buf[PATH_MAX];
 	char *name, *parent_path = NULL;
 	struct got_object_id *tree_obj_id = NULL;
 	struct got_tree_object *tree = NULL;
@@ -1862,7 +1863,10 @@ resolve_symlink(char **link_target, const char *path,
 
 	*link_target = NULL;
 
-	name = basename(path);
+	if (strlcpy(buf, path, sizeof(buf)) >= sizeof(buf))
+		return got_error(GOT_ERR_NO_SPACE);
+
+	name = basename(buf);
 	if (name == NULL)
 		return got_error_from_errno2("basename", path);
 
blob - 549cac5f15819dd3e37527467f02c394f51d5006
blob + 0648aebafecc757f25710300b9e98d53c91c585a
--- lib/path.c
+++ lib/path.c
@@ -424,9 +424,13 @@ done:
 const struct got_error *
 got_path_basename(char **s, const char *path)
 {
+	char buf[PATH_MAX];
 	char *base;
 
-	base = basename(path);
+	if (strlcpy(buf, path, sizeof(buf)) >= sizeof(buf))
+		return got_error(GOT_ERR_NO_SPACE);
+
+	base = basename(buf);
 	if (base == NULL)
 		return got_error_from_errno2("basename", path);
 
blob - 1c36cc66e308756a1e4d87c0e625f934a016272f
blob + ce156d2479af0056f65897680e9e32f8864800b6
--- lib/worktree.c
+++ lib/worktree.c
@@ -4410,13 +4410,12 @@ revert_file(void *arg, unsigned char status, unsigned 
 		if (err)
 			goto done;
 
-		te_name = basename(ie->path);
-		if (te_name == NULL) {
-			err = got_error_from_errno2("basename", ie->path);
+		err = got_path_basename(&te_name, ie->path);
+		if (err)
 			goto done;
-		}
 
 		te = got_object_tree_find_entry(tree, te_name);
+		free(te_name);
 		if (te == NULL && status != GOT_STATUS_ADD &&
 		    staged_status != GOT_STATUS_ADD) {
 			err = got_error_path(ie->path, GOT_ERR_NO_TREE_ENTRY);
@@ -4867,7 +4866,7 @@ alloc_added_blob_tree_entry(struct got_tree_entry **ne
     struct got_commitable *ct)
 {
 	const struct got_error *err = NULL;
-	char *ct_name;
+	char *ct_name = NULL;
 
 	 *new_te = NULL;
 
@@ -4875,11 +4874,9 @@ alloc_added_blob_tree_entry(struct got_tree_entry **ne
 	if (*new_te == NULL)
 		return got_error_from_errno("calloc");
 
-	ct_name = basename(ct->path);
-	if (ct_name == NULL) {
-		err = got_error_from_errno2("basename", ct->path);
+	err = got_path_basename(&ct_name, ct->path);
+	if (err)
 		goto done;
-	}
 	if (strlcpy((*new_te)->name, ct_name, sizeof((*new_te)->name)) >=
 	    sizeof((*new_te)->name)) {
 		err = got_error(GOT_ERR_NO_SPACE);
@@ -4894,6 +4891,7 @@ alloc_added_blob_tree_entry(struct got_tree_entry **ne
 	else
 		memcpy(&(*new_te)->id, ct->blob_id, sizeof((*new_te)->id));
 done:
+	free(ct_name);
 	if (err && *new_te) {
 		free(*new_te);
 		*new_te = NULL;
@@ -4997,12 +4995,15 @@ match_deleted_or_modified_ct(struct got_commitable **c
 		if (!path_matches)
 			continue;
 
-		ct_name = basename(pe->path);
-		if (ct_name == NULL)
-			return got_error_from_errno2("basename", pe->path);
+		err = got_path_basename(&ct_name, pe->path);
+		if (err)
+			return err;
 
-		if (strcmp(te->name, ct_name) != 0)
+		if (strcmp(te->name, ct_name) != 0) {
+			free(ct_name);
 			continue;
+		}
+		free(ct_name);
 
 		*ctp = ct;
 		break;