{"thread":{"id":"36181","subject":"[PATCH] index-pack: do not segfault when keep_name is NULL","startedAt":"2014-03-16T10:31:02Z","lastAt":"2014-03-17T23:07:27Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236822","messageId":"1394965862-8173-1-git-send-email-pclouds@gmail.com","threadId":"36181","inReplyTo":null,"subject":"[PATCH] index-pack: do not segfault when keep_name is NULL","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-16T10:31:02Z","receivedAt":"2014-03-16T10:31:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"keep_name is used to print error messages a couple lines down. Reset\nit to the real path returned by odb_pack_keep() if it's set to NULL by\ncaller.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n One of these moments I will make git log and friends optionally recognize\n \"Diff-Options:\" line in commit message. Adding -U14 in this case\n should help the reviewer to see how those error messages are printed.\n\n builtin/index-pack.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex a6b1c17..d95c3dc 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1283,9 +1283,10 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \tif (keep_msg) {\n \t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n \n-\t\tif (!keep_name)\n+\t\tif (!keep_name) {\n \t\t\tkeep_fd = odb_pack_keep(name, sizeof(name), sha1);\n-\t\telse\n+\t\t\tkeep_name = name;\n+\t\t} else\n \t\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n \n \t\tif (keep_fd < 0) {\n-- \n1.9.0.40.gaa8c3ea\n"},{"id":"236928","messageId":"xmqqmwgobhtm.fsf@gitster.dls.corp.google.com","threadId":"36181","inReplyTo":"1394965862-8173-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] index-pack: do not segfault when keep_name is NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T22:07:01Z","receivedAt":"2014-03-17T22:07:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> keep_name is used to print error messages a couple lines down. Reset\n> it to the real path returned by odb_pack_keep() if it's set to NULL by\n> caller.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  One of these moments I will make git log and friends optionally recognize\n>  \"Diff-Options:\" line in commit message. Adding -U14 in this case\n>  should help the reviewer to see how those error messages are printed.\n>\n>  builtin/index-pack.c | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n> index a6b1c17..d95c3dc 100644\n> --- a/builtin/index-pack.c\n> +++ b/builtin/index-pack.c\n> @@ -1283,9 +1283,10 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n>  \tif (keep_msg) {\n>  \t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n>  \n> -\t\tif (!keep_name)\n> +\t\tif (!keep_name) {\n>  \t\t\tkeep_fd = odb_pack_keep(name, sizeof(name), sha1);\n> -\t\telse\n> +\t\t\tkeep_name = name;\n> +\t\t} else\n>  \t\t\tkeep_fd = open(keep_name, O_RDWR|O_CREAT|O_EXCL, 0600);\n\nI think this fixes the right problem in a wrong way that hurts\nlonger-term maintainability.  Why not do\n\n\tkeep_name ? keep_name : name\n\nat the place where the name is used?  Otherwise you will have to\nworry about affecting later codepaths that may want to try to use\n!keep_name to switch between two codepaths, no?\n\n>  \n>  \t\tif (keep_fd < 0) {\n"},{"id":"236939","messageId":"1395097647-29362-1-git-send-email-pclouds@gmail.com","threadId":"36181","inReplyTo":"1394965862-8173-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] index-pack: do not segfault when keep_name is NULL","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-03-17T23:07:27Z","receivedAt":"2014-03-17T23:07:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Use the name that is returned by odb_pack_keep() when the caller\npasses NULL in keep_name.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/index-pack.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex a6b1c17..b9f6e12 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1291,7 +1291,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \t\tif (keep_fd < 0) {\n \t\t\tif (errno != EEXIST)\n \t\t\t\tdie_errno(_(\"cannot write keep file '%s'\"),\n-\t\t\t\t\t  keep_name);\n+\t\t\t\t\t  keep_name ? keep_name : name);\n \t\t} else {\n \t\t\tif (keep_msg_len > 0) {\n \t\t\t\twrite_or_die(keep_fd, keep_msg, keep_msg_len);\n@@ -1299,7 +1299,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \t\t\t}\n \t\t\tif (close(keep_fd) != 0)\n \t\t\t\tdie_errno(_(\"cannot close written keep file '%s'\"),\n-\t\t\t\t    keep_name);\n+\t\t\t\t\t  keep_name ? keep_name : name);\n \t\t\treport = \"keep\";\n \t\t}\n \t}\n-- \n1.9.0.40.gaa8c3ea\n"}]}