On 23-05-29 06:27PM, Stefan Sperling wrote:
> On Mon, May 29, 2023 at 04:56:25PM +0200, Stefan Sperling wrote:
> > Like this?
> 
> Omar pointed out off-list that in this diff I made the mistake of
> assigning -1 to a size_t (which is unsigned). So I would prefer to
> keep this code as-is to avoid falling into such traps.

Yes, I shouldn't have been so vague in my suggestion and shared a diff
instead. I was thinking of using the `if (len == (size_t)-1)` idiom like
below to check for asprintf() failure. But now I'm not so sure if
keeping its return value just to drop the strlen() calls are
a worthwhile optimisation. More often than not 'got rm' takes few
arguments so in the rare cases someone is dropping a large directory,
walking the string length for each path is fine. And from a cursory
look, I don't see this patten used elsewhere in got so for the sake of
consistency, it's probably better to keep it as-is.

diff /home/mark/src/got
commit - fb307946174c95e32d2048584c6ab1ce24f3ea00
path + /home/mark/src/got
blob - 9d45bcc89fba03349491b41c260b4fc0c9606531
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -4477,15 +4477,17 @@ got_worktree_schedule_delete(struct got_worktree *work
 
 	TAILQ_FOREACH(pe, paths, entry) {
 		char *ondisk_status_path;
+		size_t len;
 
-		if (asprintf(&ondisk_status_path, "%s%s%s",
+		len = asprintf(&ondisk_status_path, "%s%s%s",
 		    got_worktree_get_root_path(worktree),
-		    pe->path[0] == '\0' ? "" : "/", pe->path) == -1) {
+		    pe->path[0] == '\0' ? "" : "/", pe->path);
+		if (len == (size_t)-1) {
 			err = got_error_from_errno("asprintf");
 			goto done;
 		}
 		sda.status_path = ondisk_status_path;
-		sda.status_path_len = strlen(ondisk_status_path);
+		sda.status_path_len = len;
 		err = worktree_status(worktree, pe->path, fileindex, repo,
 			schedule_for_deletion, &sda, NULL, NULL, 1, 1);
 		free(ondisk_status_path);
@@ -9860,6 +9862,7 @@ got_worktree_patch_schedule_rm(const char *path, struc
 	const struct got_error *err;
 	struct schedule_deletion_args sda;
 	char *ondisk_status_path;
+	size_t len;
 
 	memset(&sda, 0, sizeof(sda));
 	sda.worktree = worktree;
@@ -9871,11 +9874,12 @@ got_worktree_patch_schedule_rm(const char *path, struc
 	sda.keep_on_disk = 0;
 	sda.ignore_missing_paths = 0;
 	sda.status_codes = NULL;
-	if (asprintf(&ondisk_status_path, "%s/%s",
-	    got_worktree_get_root_path(worktree), path) == -1)
+	len = asprintf(&ondisk_status_path, "%s/%s",
+	    got_worktree_get_root_path(worktree), path);
+	if (len == (size_t)-1)
 		return got_error_from_errno("asprintf");
 	sda.status_path = ondisk_status_path;
-	sda.status_path_len = strlen(ondisk_status_path);
+	sda.status_path_len = len;
 
 	err = worktree_status(worktree, path, fileindex, repo,
 	    schedule_for_deletion, &sda, NULL, NULL, 1, 1);

-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68