git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 1/1] gpg-interface: add minTrustLevel as a configuration option

From
Hans Jerry Illikainen <hji@dyntopia.com>
Date
Dec 27, 2019, 13:46 UTC
Message-ID
<87r20pkhir.hji@dyntopia.com>
In-Reply-To
<xmqqblrx5yxu.fsf@gitster-ct.c.googlers.com>
On Tue, Dec 24 2019, Junio C Hamano wrote:
Show 24 quoted lines
> Hans Jerry Illikainen <hji@dyntopia.com> writes:
>
>> +				/* Do we have trust level? */
>> +				if (sigcheck_gpg_status[i].flags & GPG_STATUS_TRUST_LEVEL) {
>> +					/*
>> +					 * GPG v1 and v2 differs in how the
>> +					 * TRUST_ lines are written.  Some
>> +					 * trust lines contain no additional
>> +					 * space-separated information for v1.
>> +					 */
>> +					next = strchr(line, ' ');
>> +					if (!next)
>> +						next = strchrnul(line, '\n');
>> +					trust = xmemdupz(line, next - line);
>
> I wonder if telling strcspn() to stop at either SP or LF is more in
> line with the existing codebase [*1*] and/or more readable.  It
> would make this part to:
>
> 		size_t trust_size = strcspn(line, " \n");
> 		trust = xmemdupz(line, trust_size);
>
> without the need to use or update the 'next' variable, if I am not
> mistaken?
I agree; fixed in v3.
Show 11 quoted lines
> By the way, while we are looking at this patch, I notice that,
> throughout the function, the use of variable 'next' feels rather
> misleading, at least to me.
>
> [...]
>
> I wonder if the code becomes less misleading if we either (1)
> renamed 'next' to a name that hints more strongly that it is not the
> 'next' line but the end of the current token we are interested in,
> or (2) get rid of the pointer and instead counted size of the
> current token we are interested in, or perhaps both?  

Yeah the name 'next' does seem a bit counter-intuitive when used in relation to 'line'. Looking through the function it seems that both (1) and (2) would work.

> This is not the fault of this patch, but I just mention it before I
> forget.
>
> Thanks.
Thanks for the review!
-- 
hji
Previous: Junio C HamanoNext: Junio C Hamano
Message 15 of 19 in “gpg-interface: add minTrustLevel as a configuration option”
  1. 0/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 16, 2019
  2. 1/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 16, 2019
  3. SZEDER GáborDec 20, 2019
  4. Hans Jerry IllikainenDec 21, 2019
  5. Randall S. BeckerDec 23, 2019
  6. Hans Jerry IllikainenDec 24, 2019
  7. Randall S. BeckerDec 24, 2019
  8. Junio C HamanoDec 16, 2019
  9. Hans Jerry IllikainenDec 18, 2019
  10. 0/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 19, 2019
  11. 1/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 19, 2019
  12. 0/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 22, 2019
  13. 1/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 22, 2019
  14. Junio C HamanoDec 24, 2019
  15. Hans Jerry IllikainenDec 27, 2019
  16. Junio C HamanoDec 27, 2019
  17. Hans Jerry IllikainenDec 22, 2019
  18. 0/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 27, 2019
  19. 1/1 gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen, Dec 27, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.