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....@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

Reply via email to