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

Re: [PATCH 4/6] revert: Make commit descriptions in insn sheet optional

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 10, 2011, 17:54 UTC
Message-ID
<7vwrccn34l.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1318095407-26429-5-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 52 quoted lines
> Change the instruction sheet format subtly so that a description of
> the commit after the object name is optional.  As a result, an
> instruction sheet like this is now perfectly valid:
>
>   pick 35b0426
>   pick fbd5bbcbc2e
>   pick 7362160f
>
> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  builtin/revert.c                |   19 ++++++++-----------
>  t/t3510-cherry-pick-sequence.sh |   14 ++++++++++++++
>  2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 6451089..aa6c34e 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -692,26 +692,23 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
>  	unsigned char commit_sha1[20];
>  	char sha1_abbrev[40];
>  	enum replay_action action;
> -	int insn_len = 0;
> -	char *p, *q;
> +	const char *p, *q;
>  
> +	p = start;
>  	if (!prefixcmp(start, "pick ")) {
>  		action = CHERRY_PICK;
> -		insn_len = strlen("pick");
> -		p = start + insn_len + 1;
> +		p += strlen("pick ");
>  	} else if (!prefixcmp(start, "revert ")) {
>  		action = REVERT;
> -		insn_len = strlen("revert");
> -		p = start + insn_len + 1;
> +		p += strlen("revert ");
>  	} else
>  		return NULL;
>  
> -	q = strchr(p, ' ');
> -	if (!q || q - p + 1 > sizeof(sha1_abbrev))
> +	q = p + strcspn(p, " \n");
> +	if (q - p + 1 > sizeof(sha1_abbrev))
>  		return NULL;
> -	q++;
> -
> -	strlcpy(sha1_abbrev, p, q - p);
> +	memcpy(sha1_abbrev, p, q - p);
> +	sha1_abbrev[q - p] = '\0';
Since this is a part of clean-up series...

Do you even need to have a sha1_abbrev[] local array that is limited to 40 bytes here? The incoming _line_ is not "const char *start", so you should at least be able to temporarily terminate the commit object name with NUL (while remembering what byte there was before), give it to get_sha1(), and then restore the byte at the end before returning from this function.

A bonus point would be to introduce a variant of get_sha1() that can take (a pointer + len) not (a pointer to NUL terminated string). While I think that would be a right thing to do in the longer term, it is outside of the scope of this series.

Previous: Ramkumar RamachandraNext: Junio C Hamano
Message 6 of 16 in “Sequencer fixups mini-series”
  1. 0/6 Sequencer fixups mini-seriesRamkumar Ramachandra, Oct 8, 2011
  2. 1/6 revert: Free memory after get_message callRamkumar Ramachandra, Oct 8, 2011
  3. 2/6 revert: Simplify getting commit subjectRamkumar Ramachandra, Oct 8, 2011
  4. 3/6 revert: Fix buffer overflow in insn sheet parserRamkumar Ramachandra, Oct 8, 2011
  5. 4/6 revert: Make commit descriptions in insn sheet optionalRamkumar Ramachandra, Oct 8, 2011
  6. Junio C HamanoOct 10, 2011
  7. Junio C HamanoOct 12, 2011
  8. Ramkumar RamachandraOct 17, 2011
  9. 5/6 revert: Allow mixed pick and revert instructionsRamkumar Ramachandra, Oct 8, 2011
  10. 6/6 revert: Simplify passing command-line arguments aroundRamkumar Ramachandra, Oct 8, 2011
  11. Tay Ray ChuanOct 9, 2011
  12. Ramkumar RamachandraOct 9, 2011
  13. Jonathan NiederOct 9, 2011
  14. Ramkumar RamachandraOct 9, 2011
  15. Jonathan NiederOct 9, 2011
  16. Ramkumar RamachandraOct 17, 2011

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.