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

From:
"Todd C. Miller" <Todd.Miller@sudo.ws>
Subject:
Re: Got relies on non-POSIX basename(3), dirname(3)
To:
Stefan Sperling <stsp@stsp.name>
Cc:
Christian Weisgerber <naddy@mips.inka.de>, gameoftrees@openbsd.org
Date:
Fri, 18 Sep 2020 12:24:59 -0600

Download raw body.

Thread
On Fri, 18 Sep 2020 20:02:50 +0200, Stefan Sperling wrote:

> So the long-term plan is to adjust OpenBSD's basename?

Yes, I think we need to go that route.

> The only potential problem I see is that hot code paths such as the
> tree object parser could end up calling malloc/free more often, which
> could hurt performance on OpenBSD. We could use fixed-size buffers on
> the stack where that matters.
>
> Using the first match found via grep as one example, in the code below
> both arguments to basename are re-used later on and the code expects
> these strings to retain their original content. So we must call strdup()
> before handing either of them to basename(3).

That looks OK but is it really any simpler than strrchr(path, '/')?
I'm admit that the base++ is a little sneaky, but the following is
nice and compact (though untested):

diff --git a/got/got.c b/got/got.c
index fff8cb02..2fc2fde6 100644
--- a/got/got.c
+++ b/got/got.c
@@ -33,7 +33,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <libgen.h>
 #include <time.h>
 #include <paths.h>
 #include <regex.h>
@@ -2454,21 +2453,9 @@ cmd_checkout(int argc, char *argv[])
 			error = got_error_from_errno("getcwd");
 			goto done;
 		}
-		if (path_prefix[0]) {
-			base = basename(path_prefix);
-			if (base == NULL) {
-				error = got_error_from_errno2("basename",
-				    path_prefix);
-				goto done;
-			}
-		} else {
-			base = basename(repo_path);
-			if (base == NULL) {
-				error = got_error_from_errno2("basename",
-				    repo_path);
-				goto done;
-			}
-		}
+		base = strrchr(path_prefix[0] ? path_prefix : repo_path, '/');
+		if (base++ == NULL)
+		    base = path_prefix[0] ? path_prefix : repo_path;
 		dotgit = strstr(base, ".git");
 		if (dotgit)
 			*dotgit = '\0';