Re: [PATCH v3 1/4] git-credential-store: support multiple credential files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 11, 2015 at 02:49:10PM +0800, Paul Tan wrote:

> Previously, git-credential-store only supported storing credentials in a
> single file: ~/.git-credentials. In order to support the XDG base
> directory specification[1], git-credential-store needs to be able to
> lookup and erase credentials from multiple files, as well as to pick the
> appropriate file to write to so that the credentials can be found on
> subsequent lookups.

I looked over the code here and in the second patch, and I didn't see
any real problems. Thanks for iterating on this.

Two minor nits, that might not even be worth addressing:

> +static void store_credential(const struct string_list *fns, struct credential *c,
> +			     const char *default_fn)

I think you could even get away without passing default_fn here, and
just use the rule "the first file in the list is the default". Unless
you are anticipating ever passing something else, but I couldn't think
of a case where that would be useful.

> +	struct string_list fns = STRING_LIST_INIT_NODUP;

This is declared NODUP...

> -	if (!file)
> +	if (file)
> +		string_list_append_nodup(&fns, file);

So you can just call the regular string_list_append here (the idea of
declaring the list as DUP or NODUP is that the callers do not have to
care; string_list_append does the right thing).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]