Johannes Sixt <j...@kdbg.org> writes:

> Am 11.09.2016 um 12:55 schrieb Johannes Schindelin:
>> -static int write_message(struct strbuf *msgbuf, const char *filename)
>> +static int write_with_lock_file(const char *filename,
>> +                            const void *buf, size_t len, int append_eol)
>>  {
>>      static struct lock_file msg_file;
>>
>>      int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
>>      if (msg_fd < 0)
>>              return error_errno(_("Could not lock '%s'"), filename);
>> -    if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
>> -            return error_errno(_("Could not write to %s"), filename);
>> -    strbuf_release(msgbuf);
>> +    if (write_in_full(msg_fd, buf, len) < 0)
>> +            return error_errno(_("Could not write to '%s'"), filename);
>> +    if (append_eol && write(msg_fd, "\n", 1) < 0)
>> +            return error_errno(_("Could not write eol to '%s"), filename);
>>      if (commit_lock_file(&msg_file) < 0)
>>              return error(_("Error wrapping up %s."), filename);
>>
>>      return 0;
>>  }
>
> The two error paths in the added lines should both
>
>               rollback_lock_file(&msg_file);
>
> , I think. But I do notice that this is not exactly new, so...

It may not be new for this step, but overall the series is aiming to
libify the stuff, so we should fix fd and lockfile leaks like this
as we notice them.

Thanks.

Reply via email to