Alberto Bertogli [off-list ref] writes:
quoted hunk
When a patch can't be opened (it doesn't exist, there are permission
problems, etc.) we get the usage text, which is not a proper indication of
failure.
This patch fixes that by simply doing a perror() instead.
Signed-off-by: Alberto Bertogli <redacted>
---
builtin-apply.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index abe73a0..d80b231 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -3120,8 +3120,11 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
arg = prefix_filename(prefix, prefix_length, arg);
fd = open(arg, O_RDONLY);
- if (fd < 0)
- usage(apply_usage);
+ if (fd < 0) {
+ perror("Error opening patch");
+ return 1;
+ }
This makes sense, but I wonder if we want to parrot "arg" back. The
problem could be that the command and the user are disagreeing which
parameter on the command line is the name of the patch file...
+
read_stdin = 0;
set_default_whitespace_mode(whitespace_option);
errs |= apply_patch(fd, arg, inaccurate_eof);
--
1.5.5.104.ge4331
When a patch can't be opened (it doesn't exist, there are permission
problems, etc.) we get the usage text, which is not a proper indication of
failure.
This patch fixes that by calling error() instead.
Signed-off-by: Alberto Bertogli <redacted>
---
This one (hopefuly) addresses both Junio and Johannes' comments.
Thanks,
Alberto
builtin-apply.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index abe73a0..ebbe609 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -3120,8 +3120,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
arg = prefix_filename(prefix, prefix_length, arg);
fd = open(arg, O_RDONLY);
- if (fd < 0)
- usage(apply_usage);
+ if (fd < 0) {
+ error("can't open patch '%s': %s", arg,
+ strerror(errno));
+ return 1;
+ }
+
read_stdin = 0;
set_default_whitespace_mode(whitespace_option);
errs |= apply_patch(fd, arg, inaccurate_eof);--
1.5.5.105.g9d11e.dirty
Hi,
On Mon, 14 Apr 2008, Alberto Bertogli wrote:
+ if (fd < 0) {
+ error("can't open patch '%s': %s", arg,
+ strerror(errno));
+ return 1;
+ }
Do you absolutely want to retain the curly braces, and have two
statements? I would prefer "return error(...)", and if you absolutely
insist on a return 1: "return !!error(...)".
Ciao,
Dscho
When a patch can't be opened (it doesn't exist, there are permission
problems, etc.) we get the usage text, which is not a proper indication of
failure.
This patch fixes that by calling error() instead.
Signed-off-by: Alberto Bertogli <redacted>
---
On Mon, Apr 14, 2008 at 03:33:54PM +0100, Johannes Schindelin wrote:
On Mon, 14 Apr 2008, Alberto Bertogli wrote:
quoted
+ if (fd < 0) {
+ error("can't open patch '%s': %s", arg,
+ strerror(errno));
+ return 1;
+ }
Do you absolutely want to retain the curly braces, and have two
statements? I would prefer "return error(...)", and if you absolutely
insist on a return 1: "return !!error(...)".
No, I'm not insisting on any version, I just thought returning 1 would be
better since it will become the script exit status; Now that I think a bit
more about it, maybe I should just use die() instead.
Anyway, here's the version returning directly from error(); if you prefer it
some other way just let me know.
Thanks,
Alberto
builtin-apply.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index abe73a0..56032ce 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -3121,7 +3121,9 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
fd = open(arg, O_RDONLY);
if (fd < 0)
- usage(apply_usage);
+ return error("can't open patch '%s': %s", arg,
+ strerror(errno));
+
read_stdin = 0;
set_default_whitespace_mode(whitespace_option);
errs |= apply_patch(fd, arg, inaccurate_eof);--
1.5.5.105.g7849.dirty