All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>, Jonathan Nieder <jrnieder@gmail.com>
Subject: [PATCH 4/8] revert: separate out parse errors logically
Date: Tue, 10 Jan 2012 21:43:55 +0530	[thread overview]
Message-ID: <1326212039-13806-5-git-send-email-artagnon@gmail.com> (raw)
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>

Three kinds of errors can arise from parsing the instruction sheet:
1. Unrecognized action
2. Malformed object name
3. Object name does not refer to a valid commit

The next patch makes an attempt to make the parser report meaningful
errors by replacing the "return -1" with "return error(...)"
appropriately.  For the first kind of error, it is insufficient to
check if the buffer beings with a "pick" or "revert", otherwise the
following insn sheet would be interpreted as having a malformed object
name:

pickle a1fe57~2

In reality, the issue is that "pickle" is an unrecognized instruction.
So, check that the buffer starts with ("pick " or "pick\t") and
("revert " or "revert\t").

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 1841ffa..9a09471 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -736,22 +736,19 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
 {
 	unsigned char commit_sha1[20];
 	char *end_of_object_name;
-	int saved, status, padding;
+	int saved, status;
 
-	if (!prefixcmp(bol, "pick")) {
+	if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
 		item->action = REPLAY_PICK;
-		bol += strlen("pick");
-	} else if (!prefixcmp(bol, "revert")) {
+		bol += strlen("pick ");
+	} else if (!prefixcmp(bol, "revert ") || !prefixcmp(bol, "revert\t")) {
 		item->action = REPLAY_REVERT;
-		bol += strlen("revert");
+		bol += strlen("revert ");
 	} else
 		return -1;
 
 	/* Eat up extra spaces/ tabs before object name */
-	padding = strspn(bol, " \t");
-	if (!padding)
-		return -1;
-	bol += padding;
+	bol += strspn(bol, " \t");
 
 	end_of_object_name = bol + strcspn(bol, " \t\n");
 	saved = *end_of_object_name;
-- 
1.7.8.2

  parent reply	other threads:[~2012-01-10 16:16 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-08 12:27 [PATCH 0/6] The move to sequencer.c Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 1/6] revert: move replay_action, replay_subcommand to header Ramkumar Ramachandra
2012-01-08 19:31   ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 2/6] revert: decouple sequencer actions from builtin commands Ramkumar Ramachandra
2012-01-08 19:34   ` Jonathan Nieder
2012-01-08 19:53     ` Ramkumar Ramachandra
2012-01-08 20:09       ` Jonathan Nieder
2012-01-08 20:07         ` Ramkumar Ramachandra
2012-01-08 20:48           ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 3/6] revert: don't let revert continue a cherry-pick Ramkumar Ramachandra
2012-01-08 19:37   ` Jonathan Nieder
2012-01-08 20:03     ` Ramkumar Ramachandra
2012-01-08 20:22       ` Jonathan Nieder
2012-01-08 20:28         ` Ramkumar Ramachandra
2012-01-08 20:45           ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 4/6] revert: allow mixing "pick" and "revert" actions Ramkumar Ramachandra
2012-01-08 19:40   ` Jonathan Nieder
2012-01-08 20:17     ` Ramkumar Ramachandra
2012-01-08 21:40       ` Jonathan Nieder
2012-01-08 21:55         ` Jonathan Nieder
2012-01-10  3:40           ` Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 5/6] revert: report fine-grained error messages from insn parser Ramkumar Ramachandra
2012-01-08 20:07   ` Jonathan Nieder
2012-01-08 20:16     ` Ramkumar Ramachandra
2012-01-08 21:33       ` Jonathan Nieder
2012-01-10 15:24         ` Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 6/6] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2012-01-08 20:38   ` Jonathan Nieder
2012-01-10 15:21     ` Ramkumar Ramachandra
2012-01-08 19:28 ` [PATCH 0/6] The move to sequencer.c Jonathan Nieder
2012-01-08 19:51   ` Ramkumar Ramachandra
2012-01-08 20:43     ` Jonathan Nieder
2012-01-10 16:13 ` [PATCH v2 0/8] " Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 1/8] revert: prepare to move replay_action to header Ramkumar Ramachandra
2012-01-10 18:27     ` Jonathan Nieder
2012-01-10 16:13   ` [PATCH 2/8] revert: decouple sequencer actions from builtin commands Ramkumar Ramachandra
2012-01-10 18:38     ` Jonathan Nieder
2012-01-11  4:02       ` Ramkumar Ramachandra
2012-01-11  4:17         ` Ramkumar Ramachandra
2012-01-11  5:04           ` Jonathan Nieder
2012-01-11  5:14             ` Ramkumar Ramachandra
2012-01-11  5:26               ` Ramkumar Ramachandra
2012-01-11  5:49                 ` Jonathan Nieder
2012-01-11  9:19                   ` Ramkumar Ramachandra
2012-01-11  9:52                     ` Jonathan Nieder
2012-01-11 10:11                       ` Ramkumar Ramachandra
2012-01-11 13:40                         ` Jonathan Nieder
2012-01-11 13:18               ` Jonathan Nieder
2012-01-11 16:39                 ` Ramkumar Ramachandra
2012-01-11 16:47                   ` Jonathan Nieder
2012-01-11 16:52                     ` Ramkumar Ramachandra
2012-01-11 18:15                     ` [PATCH v3 0/2] The move to sequencer.c Ramkumar Ramachandra
2012-01-11 18:15                       ` [PATCH 1/2] revert: prepare to move replay_action to header Ramkumar Ramachandra
2012-01-11 18:15                       ` [PATCH 2/2] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2012-01-11 18:40                       ` [PATCH v3 0/2] The move to sequencer.c Jonathan Nieder
2012-01-10 16:13   ` [PATCH 3/8] revert: allow mixing "pick" and "revert" actions Ramkumar Ramachandra
2012-01-10 16:13   ` Ramkumar Ramachandra [this message]
2012-01-10 19:03     ` [PATCH 4/8] revert: separate out parse errors logically Jonathan Nieder
2012-01-11 12:38       ` Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 5/8] revert: report fine-grained errors from insn parser Ramkumar Ramachandra
2012-01-11 12:44     ` Jonathan Nieder
2012-01-10 16:13   ` [PATCH 6/8] sha1_name: introduce getn_sha1() to take length Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 7/8] revert: use getn_sha1() to simplify insn parsing Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 8/8] sequencer: factor code out of revert builtin Ramkumar Ramachandra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1326212039-13806-5-git-send-email-artagnon@gmail.com \
    --to=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.