Tobias Klauser <tklauser@xxxxxxxxxx> writes:
> @@ -843,7 +844,9 @@ static void free_all(struct trailer_item **first)
> }
> }
>
> -void process_trailers(const char *file, int trim_empty, struct string_list *trailers)
> +static struct tempfile trailers_tempfile;
Does this need to be a static global? I'd rather have this be a local
variable of process_trailers.
> + die_errno(_("could not fdopen tempfile"));
I think you should spell it "could not open temporary file" to be more
user-friendly.
> @@ -872,5 +900,10 @@ void process_trailers(const char *file, int trim_empty, struct string_list *trai
> /* Print the lines after the trailers as is */
> print_lines(outfile, lines, trailer_end, INT_MAX);
>
> + if (in_place) {
> + if (rename_tempfile(&trailers_tempfile, file))
> + die_errno(_("could not rename tempfile"));
> + }
When this happens, I think you also want to try removing the temporary
file. Not sure, though: it may be nice to leave the tempfile for the
user to debug. What do we do in other places of the code?
It may help the user to get "could not rename temporary file %s to %s"
in case this happens.
On overall, the split makes the series much more pleasant to review, and
other than these details, this sounds good to me. Thanks!
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html