Re: [PATCH 1/2] sequencer: factor out rewrite_file()
- From
Kevin Daudt <me@ikke.info>
- Date
- Nov 1, 2017, 06:06 UTC
- Message-ID
- <20171101060608.GA1076@alpha.vpn.ikke.info>
- In-Reply-To
- <20171031163357.GA19161@alpha.vpn.ikke.info>
On Tue, Oct 31, 2017 at 05:33:57PM +0100, Kevin Daudt wrote:
Show 55 quoted lines
> 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