On Fri, Sep 18, 2020 at 12:24:59PM -0600, Todd C. Miller wrote:
> 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):

What would this do with input like "/foo///" ?

Such input probably won't happen in this particular case.
But I'd rather use an idiomatic API that understands path semantics
of data contained in the string.

Note that FreeBSD's basename() trims trailing slashes.

> 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';
>