Re: thoughts on error passing, was Re: [PATCH 2/2] fsck: handle bad trees like other errors
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 28, 2016, 18:02 UTC
- Message-ID
- <xmqqh98zkimk.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160928085841.aoisson3fnuke47q@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 27 quoted lines
> if (!dont_change_ref) {
> struct ref_transaction *transaction;
> - struct strbuf err = STRBUF_INIT;
> -
> - transaction = ref_transaction_begin(&err);
> - if (!transaction ||
> - ref_transaction_update(transaction, ref.buf,
> - sha1, forcing ? NULL : null_sha1,
> - 0, msg, &err) ||
> - ref_transaction_commit(transaction, &err))
> - die("%s", err.buf);
> +
> + transaction = ref_transaction_begin(&error_die);
> + ref_transaction_update(transaction, ref.buf,
> + sha1, forcing ? NULL : null_sha1,
> + 0, msg, &error_die);
> + ref_transaction_commit(transaction, &error_die);
> ref_transaction_free(transaction);
> - strbuf_release(&err);
> }
>
> if (real_ref && track)
>
> which is much shorter and to the point (it does rely on the called
> functions always calling report_error() and never just returning NULL or
> "-1", but that should be the already. If it isn't, we'd be printing
> "fatal: " with no message).Yes but... grepping for die() got a lot harder, which may not be a good thing.
I do like the flexibility such a mechanism offers, but wrapping/hiding die in it is probably an example that the flexibility went a bit too far.