Re: [PATCH v2 21/25] sequencer: refactor write_message()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio & Hannes,

On Thu, 15 Sep 2016, Junio C Hamano wrote:

> Johannes Sixt <j6t@xxxxxxxx> 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.

Makes sense, even for the final commit_lock_file().

Ciao,
Dscho



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]