Re: [PATCH] interpret-trailers: add option for in-place editing
- From
Tobias Klauser <tklauser@distanz.ch>
- Date
- Jan 7, 2016, 12:42 UTC
- Message-ID
- <20160107124216.GK29125@distanz.ch>
- In-Reply-To
- <CAPig+cSvD-TAd6365wxycUcTBehL-81a0WWosz4mVr1-HKCqGQ@mail.gmail.com>
On 2016-01-06 at 20:02:23 +0100, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 33 quoted lines
> On Wed, Jan 6, 2016 at 8:34 AM, Tobias Klauser
> <tobias.klauser@zhinst.com> wrote:
> > Add a command line option --in-place to support in-place editing akin to
> > sed -i. This allows to write commands like the following:
> >
> > git interpret-trailers --trailer "X: Y" a.txt > b.txt && mv b.txt a.txt
> >
> > in a more concise way:
> >
> > git interpret-trailers --trailer "X: Y" --in-place a.txt
> >
> > Also add the corresponding documentation and tests.
>
> In addition to Matthieu's comments...
>
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> > diff --git a/trailer.c b/trailer.c
> > @@ -856,19 +858,28 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai
> >
> > lines = read_input_file(file);
> >
> > + if (in_place) {
> > + fp = fopen(file, "w");
> > + if (!fp)
> > + die_errno(_("could not open file '%s' for writing"), file);
> > + }
>
> The input file should be considered precious, but this implementation
> plays too loosely with it. If the user interrupts the program or a
> die() somewhere within the "trailers" code aborts the program before
> the output file is written, then the original file will be
> irrecoverably lost. Users won't be happy about that.Indeed, I didn't consider this. Thanks a lot for pointing this out.
> Instead, this code should go through the standard dance of writing the > output to a new file (with some unique temporary name) and then, only > once the output has been successfully written in full, rename the new > file atop the old.
Ok, will do this for v2. I guess with the help of the functions from tempfile.h it should be fairly easy to implement...
Thanks for your review!