"André Goddard Rosa" <andre.goddard@xxxxxxxxx> writes:
> On Nov 22, 2007 2:09 PM, Alex Riesen <raa.lkml@xxxxxxxxx> wrote:
> ...
> I tested it here before posting but luckly (or not, as I didn't catch
> this when compiling) it worked,
> as a pointer have the sizeof(int) in my x86 platform. >:|
Most of the changes look trivially correct. Thanks.
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index be9e3ea..84c8ed4 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -263,8 +263,13 @@ static void store_updated_refs(const char *url,
> struct ref *ref_map)
> char note[1024];
> const char *what, *kind;
> struct ref *rm;
> + char *filename = git_path("FETCH_HEAD");
>
> - fp = fopen(git_path("FETCH_HEAD"), "a");
> + fp = fopen(filename, "a");
> + if (!fp) {
> + error("cannot open %s: %s\n", filename, strerror(errno));
> + return 1;
> + }
> for (rm = ref_map; rm; rm = rm->next) {
> struct ref *ref = NULL;
I started to wonder if there was a particular reason you chose to return
1, not -1 (or just say 'return error("cannot open...", ...)')?
> @@ -404,7 +410,7 @@ static int fetch_refs(struct transport *transport,
> struct ref *ref_map)
> if (ret)
> ret = transport_fetch_refs(transport, ref_map);
> if (!ret)
> - store_updated_refs(transport->url, ref_map);
> + ret |= store_updated_refs(transport->url, ref_map);
> transport_unlock_pack(transport);
> return ret;
> }
I think the callers of fetch_refs() are interested in seeing only 0 or
non-zero, so it seems more consistent to signal error with negative
return. I modified the hunk that starts at line 263 to return the
return value from error() and applied.
That made me follow the transport code, fetch_refs_via_pack(). This is
not your code, so Shawn and Daniel are CC'ed.
The code calls fetch_pack() to get the list of refs it fetched, and
discards refs and always returns 0 to signal success.
But builtin-fetch-pack.c::fetch_pack() has error cases. The function
returns NULL if error is detected (shallow-support side seems to choose
to die but I suspect that is easily fixable to error out as well).
Shouldn't fetch_refs_via_pack() propagate that error to the caller?
---
transport.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/transport.c b/transport.c
index 50db980..048df1f 100644
--- a/transport.c
+++ b/transport.c
@@ -655,7 +655,7 @@ static int fetch_refs_via_pack(struct transport *transport,
free(heads);
free_refs(refs);
free(dest);
- return 0;
+ return (refs ? 0 : -1);
}
static int git_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags)
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
