git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Potapov <dpotapov@gmail.com>
To: Johannes Sixt <j.sixt@viscovery.net>, Junio C Hamano <gitster@pobox.com>
Cc: "Shawn O. Pearce" <spearce@spearce.org>, git@vger.kernel.org
Subject: [PATCH] print an error message for invalid path
Date: Sat, 11 Oct 2008 20:39:37 +0400	[thread overview]
Message-ID: <20081011163937.GA21650@dpotapov.dyndns.org> (raw)
In-Reply-To: <48EAFBC2.7020305@viscovery.net>

If verification of path failed, it is always better to print an error message
saying this than relying on the caller function to print a meaningful error
message (especially when the callee already prints error message for another
situation).

Because the callers of add_index_entry_with_check() did not print any error
message, it resulted that the user would not notice the problem when checkout
if an invalid path failed.

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---

On Tue, Oct 07, 2008 at 08:03:46AM +0200, Johannes Sixt wrote:
> 
> Look at the original patch. You did not change the behavior except to
> write more error messages. Maybe I misunderstand the words "to error out".
> I understand them as "to detect an error and return early", but not "write
> an error message".

For me, to "error out" means to show an error to the user. Usually, it
implies that the program will return after that, though not necessary
immediately. (Like gcc may print an error but it continues to parse the
program and may report more errors).

You are right that I have not changed anything in terms of exiting
earlier, and because I am aware about any commonly accepted definition
of what "error out" means, I have replaced the comment with less
ambiguous and detail description.


 builtin-update-index.c |    2 +-
 read-cache.c           |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin-update-index.c b/builtin-update-index.c
index 417f972..3a2291b 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -218,7 +218,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
 	struct cache_entry *ce;
 
 	if (!verify_path(path))
-		return -1;
+		return error("Invalid path '%s'", path);
 
 	len = strlen(path);
 	size = cache_entry_size(len);
diff --git a/read-cache.c b/read-cache.c
index 901064b..aff6390 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -591,8 +591,10 @@ struct cache_entry *make_cache_entry(unsigned int mode,
 	int size, len;
 	struct cache_entry *ce;
 
-	if (!verify_path(path))
+	if (!verify_path(path)) {
+		error("Invalid path '%s'", path);
 		return NULL;
+	}
 
 	len = strlen(path);
 	size = cache_entry_size(len);
@@ -874,7 +876,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
 	if (!ok_to_add)
 		return -1;
 	if (!verify_path(ce->name))
-		return -1;
+		return error("Invalid path '%s'", ce->name);
 
 	if (!skip_df_check &&
 	    check_file_directory_conflict(istate, ce, pos, ok_to_replace)) {
-- 
1.6.0.2.447.g64b01

      reply	other threads:[~2008-10-11 16:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-05  0:40 [PATCH] make prefix_path() never return NULL Dmitry Potapov
2008-10-05  2:14 ` [PATCH] do not segfault if make_cache_entry failed Dmitry Potapov
2008-10-05  2:14   ` [PATCH] error out if path is invalid Dmitry Potapov
2008-10-06  7:02     ` Johannes Sixt
2008-10-07  0:22       ` Dmitry Potapov
2008-10-07  6:03         ` Johannes Sixt
2008-10-11 16:39           ` Dmitry Potapov [this message]

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=20081011163937.GA21650@dpotapov.dyndns.org \
    --to=dpotapov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=spearce@spearce.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).