Torsten Bögershausen wrote:
> On 12/03/2014 06:14 AM, Jonathan Nieder wrote:
>> --- a/lockfile.c
>> +++ b/lockfile.c
>> @@ -179,45 +179,36 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
>> return fd;
>> }
>> -int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
>> +int hold_lock_file_for_append(struct lock_file *lk, const char *path,
>> + int flags, struct strbuf *err)
>> {
>> int fd, orig_fd;
>> - struct strbuf err = STRBUF_INIT;
>> +
>> + assert(!(flags & LOCK_DIE_ON_ERROR));
>> + assert(err && !err->len);
>
> What do I miss ?
> In the old code we die() printing the errno when LOCK_DIE_ON_ERRORis set.
The assertion documents an assumption that no caller will set the
LOCK_DIE_ON_ERROR flag bit. A later patch in the series eliminates
that flag bit completely.
> Now we die() in an assert() or two ?
assert() is not safe to use for anything other than documenting
assumptions. It can't be relied on to exit (let alone to exit with a
nice message like die()). So the die() that was removed and assert()
that this patch adds have different purposes.
> And should'nt we assert in strbuf_addf() instead ?
strbuf_addf can be used to append to a nonempty strbuf.
[...]
> (Unless it will be used in later commit, and the we could mention it here)
Good idea. I'll add to the commit message if rerolling.
Thanks,
Jonathan
--
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