logmsg_reencode: never return NULL

The logmsg_reencode function will return the reencoded commit buffer, or NULL if reencoding failed or no reencoding was necessary. Since every caller then ends up checking for NULL and just using the commit's original buffer, anyway, we can be a bit more helpful and just return that buffer when we would have returned NULL. Since the resulting string may or may not need to be freed, we introduce a logmsg_free, which checks whether the buffer came from the commit object or not (callers either implemented the same check already, or kept two separate pointers, one to mark the buffer to be used, and one for the to-be-freed string). Pushing this logic into logmsg_* simplifies the callers, and will let future patches lazily load the commit buffer in a single place. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 26, 2013 at 04:44 UTC dd0d388c44c28ebc021a24eeddc60287d4ea249c
4 files changed +29 -33
builtin/blame.c
+4 -5
@@ -1420,7 +1420,7 @@ static void get_commit_info(struct commit *commit,
1420 {
1421 int len;
1422 const char *subject, *encoding;
1423 - char *reencoded, *message;
1423 + char *message;
1424
1425 commit_info_init(ret);
1426
@@ -1438,14 +1438,13 @@ static void get_commit_info(struct commit *commit,
1438 sha1_to_hex(commit->object.sha1));
1439 }
1440 encoding = get_log_output_encoding();
1441 - reencoded = logmsg_reencode(commit, encoding);
1442 - message = reencoded ? reencoded : commit->buffer;
1441 + message = logmsg_reencode(commit, encoding);
1442 get_ac_line(message, "\nauthor ",
1443 &ret->author, &ret->author_mail,
1444 &ret->author_time, &ret->author_tz);
1445
1446 if (!detailed) {
1448 - free(reencoded);
1447 + logmsg_free(message, commit);
1448 return;
1449 }
1450
@@ -1459,7 +1458,7 @@ static void get_commit_info(struct commit *commit,
1458 else
1459 strbuf_addf(&ret->summary, "(%s)", sha1_to_hex(commit->object.sha1));
1460
1462 - free(reencoded);
1461 + logmsg_free(message, commit);
1462 }
1463
1464 /*
builtin/commit.c
+2 -12
@@ -946,24 +946,14 @@ static void handle_untracked_files_arg(struct wt_status *s)
946
947 static const char *read_commit_message(const char *name)
948 {
949 - const char *out_enc, *out;
949 + const char *out_enc;
950 struct commit *commit;
951
952 commit = lookup_commit_reference_by_name(name);
953 if (!commit)
954 die(_("could not lookup commit %s"), name);
955 out_enc = get_commit_output_encoding();
956 - out = logmsg_reencode(commit, out_enc);
957 -
958 - /*
959 - * If we failed to reencode the buffer, just copy it
960 - * byte for byte so the user can try to fix it up.
961 - * This also handles the case where input and output
962 - * encodings are identical.
963 - */
964 - if (out == NULL)
965 - out = commit->buffer;
966 - return out;
956 + return logmsg_reencode(commit, out_enc);
957 }
958
959 static int parse_and_validate_options(int argc, const char *argv[],
commit.h
+1
@@ -101,6 +101,7 @@ extern int has_non_ascii(const char *text);
101 struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
102 extern char *logmsg_reencode(const struct commit *commit,
103 const char *output_encoding);
104 +extern void logmsg_free(char *msg, const struct commit *commit);
105 extern void get_commit_format(const char *arg, struct rev_info *);
106 extern const char *format_subject(struct strbuf *sb, const char *msg,
107 const char *line_separator);
pretty.c
+22 -16
@@ -524,10 +524,11 @@ static void add_merge_info(const struct pretty_print_context *pp,
524 strbuf_addch(sb, '\n');
525 }
526
527 -static char *get_header(const struct commit *commit, const char *key)
527 +static char *get_header(const struct commit *commit, const char *msg,
528 + const char *key)
529 {
530 int key_len = strlen(key);
530 - const char *line = commit->buffer;
531 + const char *line = msg;
532
533 while (line) {
534 const char *eol = strchr(line, '\n'), *next;
@@ -588,17 +589,18 @@ char *logmsg_reencode(const struct commit *commit,
589 static const char *utf8 = "UTF-8";
590 const char *use_encoding;
591 char *encoding;
592 + char *msg = commit->buffer;
593 char *out;
594
595 if (!output_encoding || !*output_encoding)
594 - return NULL;
595 - encoding = get_header(commit, "encoding");
596 + return msg;
597 + encoding = get_header(commit, msg, "encoding");
598 use_encoding = encoding ? encoding : utf8;
599 if (same_encoding(use_encoding, output_encoding))
600 if (encoding) /* we'll strip encoding header later */
601 out = xstrdup(commit->buffer);
602 else
601 - return NULL; /* nothing to do */
603 + return msg; /* nothing to do */
604 else
605 out = reencode_string(commit->buffer,
606 output_encoding, use_encoding);
@@ -606,7 +608,17 @@ char *logmsg_reencode(const struct commit *commit,
608 out = replace_encoding_header(out, output_encoding);
609
610 free(encoding);
609 - return out;
611 + /*
612 + * If the re-encoding failed, out might be NULL here; in that
613 + * case we just return the commit message verbatim.
614 + */
615 + return out ? out : msg;
616 +}
617 +
618 +void logmsg_free(char *msg, const struct commit *commit)
619 +{
620 + if (msg != commit->buffer)
621 + free(msg);
622 }
623
624 static int mailmap_name(const char **email, size_t *email_len,
@@ -1278,14 +1290,11 @@ void format_commit_message(const struct commit *commit,
1290 context.pretty_ctx = pretty_ctx;
1291 context.wrap_start = sb->len;
1292 context.message = logmsg_reencode(commit, output_enc);
1281 - if (!context.message)
1282 - context.message = commit->buffer;
1293
1294 strbuf_expand(sb, format, format_commit_item, &context);
1295 rewrap_message_tail(sb, &context, 0, 0, 0);
1296
1287 - if (context.message != commit->buffer)
1288 - free(context.message);
1297 + logmsg_free(context.message, commit);
1298 free(context.signature.gpg_output);
1299 free(context.signature.signer);
1300 }
@@ -1432,7 +1441,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
1441 {
1442 unsigned long beginning_of_body;
1443 int indent = 4;
1435 - const char *msg = commit->buffer;
1444 + const char *msg;
1445 char *reencoded;
1446 const char *encoding;
1447 int need_8bit_cte = pp->need_8bit_cte;
@@ -1443,10 +1452,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
1452 }
1453
1454 encoding = get_log_output_encoding();
1446 - reencoded = logmsg_reencode(commit, encoding);
1447 - if (reencoded) {
1448 - msg = reencoded;
1449 - }
1455 + msg = reencoded = logmsg_reencode(commit, encoding);
1456
1457 if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
1458 indent = 0;
@@ -1503,7 +1509,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
1509 if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
1510 strbuf_addch(sb, '\n');
1511
1506 - free(reencoded);
1512 + logmsg_free(reencoded, commit);
1513 }
1514
1515 void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,