Re: [PATCH 1/2] sequencer: factor out rewrite_file()

[Kevin Daudt <me@ikke.info>]

On Tue, Oct 31, 2017 at 05:33:57PM +0100, Kevin Daudt wrote:
> On Tue, Oct 31, 2017 at 10:54:21AM +0100, René Scharfe wrote:
> > Reduce code duplication by extracting a function for rewriting an
> > existing file.
> > 
> > Signed-off-by: Rene Scharfe <l.s.r@web.de>
> > ---
> >  sequencer.c | 46 +++++++++++++++++-----------------------------
> >  1 file changed, 17 insertions(+), 29 deletions(-)
> > 
> > diff --git a/sequencer.c b/sequencer.c > > index f2a10cc4f2..17360eb38a 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -2665,6 +2665,20 @@ int check_todo_list(void)
> >  	return res;
> >  }
> >  
> > +static int rewrite_file(const char *path, const char *buf, size_t len)
> > +{
> > +	int rc = 0;
> > +	int fd = open(path, O_WRONLY);
> > +	if (fd < 0)
> > +		return error_errno(_("could not open '%s' for writing"), path);
> > +	if (write_in_full(fd, buf, len) < 0)
> > +		rc = error_errno(_("could not write to '%s'"), path);
> > +	if (!rc && ftruncate(fd, len) < 0)
> > +		rc = error_errno(_("could not truncate '%s'"), path);
> > +	close(fd);
> > +	return rc;
> > +}
> > +
> >  /* skip picking commits whose parents are unchanged */
> >  int skip_unnecessary_picks(void)
> >  {
> > @@ -2737,29 +2751,11 @@ int skip_unnecessary_picks(void)
> >  		}
> >  		close(fd);
> >  
> > -		fd = open(rebase_path_todo(), O_WRONLY, 0666);
> > -		if (fd < 0) {
> > -			error_errno(_("could not open '%s' for writing"),
> > -				    rebase_path_todo());
> > +		if (rewrite_file(rebase_path_todo(), todo_list.buf.buf + offset,
> > +				 todo_list.buf.len - offset) < 0) {
> >  			todo_list_release(&todo_list);
> >  			return -1;
> >  		}
> > -		if (write_in_full(fd, todo_list.buf.buf + offset,
> > -				todo_list.buf.len - offset) < 0) {
> > -			error_errno(_("could not write to '%s'"),
> > -				    rebase_path_todo());
> > -			close(fd);
> > -			todo_list_release(&todo_list);
> 
> Is this missing on purpose in the new situation?
>

I wasn't looking at the context, only the changed lines. After reading
it again, it's clear that nothing is missing (the freeing of todo_list).

Kevin