Re: [PATCH 15/27] sha1_file: allow prepare_alt_odb to handle arbitrary repositories

2018-02-23 Thread Stefan Beller
On Wed, Feb 21, 2018 at 4:35 PM, Jonathan Tan  wrote:
> On Tue, 20 Feb 2018 17:54:18 -0800
> Stefan Beller  wrote:
>
>> -void prepare_alt_odb_the_repository(void)
>> +void prepare_alt_odb(struct repository *r)
>>  {
>> - const char *alt;
>> -
>> - if (the_repository->objects.alt_odb_tail)
>> + if (r->objects.alt_odb_tail)
>>   return;
>>
>> - alt = getenv(ALTERNATE_DB_ENVIRONMENT);
>> + r->objects.alt_odb_tail = >objects.alt_odb_list;
>> +
>> + if (!r->ignore_env) {
>> + const char *alt = getenv(ALTERNATE_DB_ENVIRONMENT);
>> + if (!alt)
>> + alt = "";
>
> alt can be NULL, just like in the existing code, so these 2 lines are
> unnecessary. (link_alt_odb_entries() will just do nothing in either
> case.)
>
> (I also think that the check of absence of alt should be done by the
> caller of link_alt_odb_entries(), not by link_alt_odb_entries() itself,
> but that is much beyond the scope of this patch set.)
>

reverted to a literal translation of s/the_repository/r/, I forget
why I implemented this change in the first place. dropped for now.

Thanks,
Stefan


Re: [PATCH 15/27] sha1_file: allow prepare_alt_odb to handle arbitrary repositories

2018-02-21 Thread Jonathan Tan
On Tue, 20 Feb 2018 17:54:18 -0800
Stefan Beller  wrote:

> -void prepare_alt_odb_the_repository(void)
> +void prepare_alt_odb(struct repository *r)
>  {
> - const char *alt;
> -
> - if (the_repository->objects.alt_odb_tail)
> + if (r->objects.alt_odb_tail)
>   return;
>  
> - alt = getenv(ALTERNATE_DB_ENVIRONMENT);
> + r->objects.alt_odb_tail = >objects.alt_odb_list;
> +
> + if (!r->ignore_env) {
> + const char *alt = getenv(ALTERNATE_DB_ENVIRONMENT);
> + if (!alt)
> + alt = "";

alt can be NULL, just like in the existing code, so these 2 lines are
unnecessary. (link_alt_odb_entries() will just do nothing in either
case.)

(I also think that the check of absence of alt should be done by the
caller of link_alt_odb_entries(), not by link_alt_odb_entries() itself,
but that is much beyond the scope of this patch set.)

>  
> - the_repository->objects.alt_odb_tail =
> - _repository->objects.alt_odb_list;
> - link_alt_odb_entries(the_repository, alt,
> -  PATH_SEP, NULL, 0);
> + link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0);
> + }
>  
> - read_info_alternates(the_repository, get_object_directory(), 0);
> + read_info_alternates(r, r->objects.objectdir, 0);
>  }