Stefan Beller <sbeller@xxxxxxxxxx> writes:
> Having the return value inverted we can have different values for
> the error codes. This is useful in a later patch when we want to
> know if we hit the REF_STATUS_REJECT_* case.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
>
> Notes:
> New in the series. For consistency with all the other patches
> it also reads v2.
Sorry, but ECANNOTPARSE especially "it also read v2" part.
> send-pack.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/send-pack.c b/send-pack.c
> index 2a513f4..1c4ac75 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -190,10 +190,10 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
> for_each_commit_graft(advertise_shallow_grafts_cb, sb);
> }
>
> -static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
> +static int no_ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
> {
> if (!ref->peer_ref && !args->send_mirror)
> - return 0;
> + return 1;
>
> /* Check for statuses set by set_ref_status_for_push() */
> switch (ref->status) {
> @@ -203,10 +203,11 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
> case REF_STATUS_REJECT_NEEDS_FORCE:
> case REF_STATUS_REJECT_STALE:
> case REF_STATUS_REJECT_NODELETE:
> + return 2;
> case REF_STATUS_UPTODATE:
> - return 0;
> + return 3;
> default:
> - return 1;
> + return 0;
Hmmm. It used to be "will we send this one?" It is fine if the
function wants to differenciate various kinds of error, but
(1) unexplained 1, 2 and 3 looks cryptic and unmaintainable;
(2) no_ prefix in the function name is usually a bad idea, because
it easily invites "if (!no_foo(...))" double negation; and
(3) unless there is a good reason to do otherwise, a function that
returns 0 on success should signal an error with a negative
return.
"static int check_to_send_update()" or something, perhaps?
if (check_to_send_update() < 0)
/* skip as this is an error */
continue;
does not look too bad.
> }
>
> @@ -250,7 +251,7 @@ static int generate_push_cert(struct strbuf *req_buf,
> strbuf_addstr(&cert, "\n");
>
> for (ref = remote_refs; ref; ref = ref->next) {
> - if (!ref_update_to_be_sent(ref, args))
> + if (no_ref_update_to_be_sent(ref, args))
> continue;
> update_seen = 1;
> strbuf_addf(&cert, "%s %s %s\n",
> @@ -370,7 +371,7 @@ int send_pack(struct send_pack_args *args,
> * the pack data.
> */
> for (ref = remote_refs; ref; ref = ref->next) {
> - if (!ref_update_to_be_sent(ref, args))
> + if (no_ref_update_to_be_sent(ref, args))
> continue;
>
> if (!ref->deletion)
> @@ -391,7 +392,7 @@ int send_pack(struct send_pack_args *args,
> if (args->dry_run || args->push_cert)
> continue;
>
> - if (!ref_update_to_be_sent(ref, args))
> + if (no_ref_update_to_be_sent(ref, args))
> continue;
>
> old_hex = sha1_to_hex(ref->old_sha1);
--
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