Re: [PATCH v5 3/5] pseudorefs: create and use pseudoref update and delete functions

From: Stefan Beller <sbeller@google.com>
Date: Jul 31, 2015, 23:40 UTC
Message-ID: <CAGZ79kZ-e-GU5FfKWDAUxQPUnyrmGjSvHGnjWn=pwpDAj37umQ@mail.gmail.com>
In-Reply-To: <1438322781-21181-3-git-send-email-dturner@twopensource.com>

I am sorry for being late to the review, I looked into coverity today as Duy bugged me to fix the memory allocation stuff[1]

[1] $gmane/275046

On Thu, Jul 30, 2015 at 11:06 PM, David Turner <dturner@twopensource.com> wrote:

> +
> +       if (old_sha1) {
> +               unsigned char actual_old_sha1[20];
> +               read_ref(pseudoref, actual_old_sha1);

What about the return value of read_ref? In most cases of the code base (19/21) we check the return of that. So maybe

    if (read_ref(pseudoref, actual_old_sha1) <0)
        die("Could not read ref %s", pseudoref);

> +               if (fd < 0)
> +                       die_errno(_("Could not open '%s' for writing"), filename);
> +               read_ref(pseudoref, actual_old_sha1);

same here.