From: Martijn van Duren <openbsd+got@list.imperialat.at>
Subject: Re: avoid per tree-entry asprintf/free
To: Omar Polo <op@omarpolo.com>, Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Tue, 24 Feb 2026 22:07:27 +0100

On 2/24/26 21:47, Omar Polo wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
>> Avoid doing asprintf/free per tree entry in got_pack_load_tree_entries().
>>  
>> Allocate one path buffer and keep growing it as needed. For directories we
>> still need to copy the path. But blobs can use the path buffer directly.
>>
>> ok?
> 
> okay op with a nit:
> 
>> M  lib/pack_create.c  |  38+  11-
>>
>> 1 file changed, 38 insertions(+), 11 deletions(-)
>>
>> commit - 23c16b295963fd2cfddf454ef642b1dfb6afa3d1
>> commit + bf9514555f3e1bcbc972ed5f791d2cf0a9c797e6
>> blob - 88ae6a7f7857b14079689f7c4e8f26ef6cebdfce
>> blob + 0e3b3d8caf3352ddb1c89e6e64acc9d0897aa079
>> --- lib/pack_create.c
>> +++ lib/pack_create.c
>> @@ -797,7 +797,7 @@ got_pack_load_tree_entries(struct got_object_id_queue 
>>  {
>>  	const struct got_error *err;
>>  	char *p = NULL;
>> -	int i;
>> +	int i, ret, psize = _POSIX_PATH_MAX;
> 
> can we use PATH_MAX instead?
> 
> 	$ grep -R PATH_MAX | grep -vc _POSIX
> 	88
> 	$ grep -R POSIX_PATH_MAX | wc -l
> 	15
>       
> we seem to have a preference for PATH_MAX and, as far as I know, unlike
> HOST_NAME_MAX which is problematic in portable, PATH_MAX is generally
> okay.
> 
> (plus I'd like to avoid these _POSIX_* spelling if possible)

PATH_MAX is part of POSIX, with a Minimum Acceptable Value of
_POSIX_PATH_MAX, where _POSIX_PATH_MAX is set to Value: 256.[0]

So one option could be to not accept paths inside a repo that're longer
than _POSIX_PATH_MAX, so that every OS should be able to extract it
(even if only under /), but use PATH_MAX for in memory.
Another option could be to always use PATH_MAX, and say that maximum
compat is with the OS with the largest PATH_MAX, and if you have a
repo with paths more than PATH_MAX bytes, just buy yourself a better
OS.

Regardless of choice, I think these two options should be the foundation
of choosing one over the other.

martijn@

> 
>>  	(*ntrees)++;
>>  	err = got_pack_report_progress(progress_cb, progress_arg, rl,
> 

[0] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html