On Thu, Nov 20, 2014 at 10:10 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> If we don't pass in the error string buffer, we skip over all
> parts dealing with preparing error messages.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
>
> This goes ontop of [PATCH v5] refs.c: use a stringlist for repack_without_refs
> if that makes sense.
>
> refs.c | 8 ++++----
> refs.h | 1 -
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index ebcd90f..3c85ea6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2646,8 +2646,6 @@ int repack_without_refs(struct string_list *without, struct strbuf *err)
> struct string_list_item *ref_to_delete;
> int ret, needs_repacking = 0, removed = 0;
>
> - assert(err);
> -
> /* Look for a packed ref */
> for_each_string_list_item(ref_to_delete, without) {
> if (get_packed_ref(ref_to_delete->string)) {
> @@ -2661,7 +2659,9 @@ int repack_without_refs(struct string_list *without, struct strbuf *err)
> return 0;
>
> if (lock_packed_refs(0)) {
> - unable_to_lock_message(git_path("packed-refs"), errno, err);
> + if (err)
> + unable_to_lock_message(git_path("packed-refs"),
> + errno, err);
> return -1;
> }
> packed = get_packed_refs(&ref_cache);
> @@ -2688,7 +2688,7 @@ int repack_without_refs(struct string_list *without, struct strbuf *err)
>
> /* Write what remains */
> ret = commit_packed_refs();
> - if (ret)
> + if (ret && err)
> strbuf_addf(err, "unable to overwrite old ref-pack file: %s",
> strerror(errno));
> return ret;
> diff --git a/refs.h b/refs.h
> index c7323ff..b71fb79 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -170,7 +170,6 @@ int pack_refs(unsigned int flags);
> * strbuf.
> *
> * The refs in 'without' may have any order.
> - * The err buffer must not be omitted.
> */
> extern int repack_without_refs(struct string_list *without, struct strbuf *err);
>
> --
> 2.2.0.rc2.23.gca0107e
>
LGTM
Reviewed-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
Nit:
While it does not hurt to allow passing NULL, at some stage later
this function will become
private to refs.c and ONLY be called from within transaction_commit()
which will always
pass a non-NULL err argument.
At that stage we will not strictly need to allow err==NULL since all
callers are guaranteed to
always pass err!=NULL.
That said, having err being optional is probably a better API. Maybe
err should be made optional for all other functions that take
an err strbuf too so that the calling conventions become more consistent?
--
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