Re: [PATCH v8 10/11] convert: make apply_filter() adhere to standard Git error handling

[Jakub Narębski]

W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:
> From: Lars Schneider 
> 
> apply_filter() returns a boolean that tells the caller if it
> "did convert or did not convert". The variable `ret` was used throughout
> the function to track errors whereas `1` denoted success and `0`
> failure. This is unusual for the Git source where `0` denotes success.
> 
> Rename the variable and flip its value to make the function easier
> readable for Git developers.

This also allow to use the 'err = error("");' idiom,
isn't it...

> 
> Signed-off-by: Lars Schneider 
> ---
>  convert.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)

...which allows to delete some lines of code.  Very nice.

> - int ret = 1;
> + int err = 0;

> - error("read from external filter '%s' failed", cmd);
> - ret = 0;
> + err = error("read from external filter '%s' failed", cmd);

> - if (ret) {
> + if (!err) {

> - return ret;
> + return !err;

Looks good.

[PATCH v8 10/11] convert: make apply_filter() adhere to standard Git error handling

[larsxschneider]

From: Lars Schneider 

apply_filter() returns a boolean that tells the caller if it
"did convert or did not convert". The variable `ret` was used throughout
the function to track errors whereas `1` denoted success and `0`
failure. This is unusual for the Git source where `0` denotes success.

Rename the variable and flip its value to make the function easier
readable for Git developers.

Signed-off-by: Lars Schneider 
---
 convert.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/convert.c b/convert.c
index 986c239..597f561 100644
--- a/convert.c
+++ b/convert.c
@@ -451,7 +451,7 @@ static int apply_filter(const char *path, const char *src, 
size_t len, int fd,
 *
 * (child --> cmd) --> us
 */
-   int ret = 1;
+   int err = 0;
struct strbuf nbuf = STRBUF_INIT;
struct async async;
struct filter_params params;
@@ -477,23 +477,20 @@ static int apply_filter(const char *path, const char 
*src, size_t len, int fd,
return 0;   /* error was already reported */
 
if (strbuf_read(, async.out, len) < 0) {
-   error("read from external filter '%s' failed", cmd);
-   ret = 0;
+   err = error("read from external filter '%s' failed", cmd);
}
if (close(async.out)) {
-   error("read from external filter '%s' failed", cmd);
-   ret = 0;
+   err = error("read from external filter '%s' failed", cmd);
}
if (finish_async()) {
-   error("external filter '%s' failed", cmd);
-   ret = 0;
+   err = error("external filter '%s' failed", cmd);
}
 
-   if (ret) {
+   if (!err) {
strbuf_swap(dst, );
}
strbuf_release();
-   return ret;
+   return !err;
 }
 
 static struct convert_driver {
-- 
2.10.0