{"thread":{"id":"61984","subject":"[PATCH] mailinfo: we parse fixed headers","startedAt":"2024-08-20T21:09:15Z","lastAt":"2024-08-22T08:24:17Z","messageCount":2,"participants":["Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"501395","messageId":"xmqq1q2i6gw7.fsf@gitster.g","threadId":"61984","inReplyTo":null,"subject":"[PATCH] mailinfo: we parse fixed headers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-20T21:09:12Z","receivedAt":"2024-08-20T21:09:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The code was written as if we have a small room to add additional\nheaders to be parsed to the header[] array at runtime, but that is\nnot our intention at all.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * This will not be applied and merged down, as this will conflict\n   with Patrick's leakfix part 5.  In this version, MAX_HDR_PARSED\n   is still set to 1 larger than necessary, due to the use of\n   strbuf_list_free(), but after the leakfix-5 topic fixes it,\n   MAX_HDR_PARSED can be reduced to ARRAY_SIZE(header).\n\n mailinfo.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git c/mailinfo.c w/mailinfo.c\nindex 94b9b0abf2..17d7c3b594 100644\n--- c/mailinfo.c\n+++ w/mailinfo.c\n@@ -346,10 +346,10 @@ static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)\n \tstrbuf_trim(subject);\n }\n \n-#define MAX_HDR_PARSED 10\n-static const char *header[MAX_HDR_PARSED] = {\n-\t\"From\",\"Subject\",\"Date\",\n+static const char * const header[] = {\n+\t\"From\", \"Subject\", \"Date\",\n };\n+#define MAX_HDR_PARSED (ARRAY_SIZE(header) + 1)\n \n static inline int skip_header(const struct strbuf *line, const char *hdr,\n \t\t\t      const char **outval)\n@@ -583,7 +583,7 @@ static int check_header(struct mailinfo *mi,\n \tstruct strbuf sb = STRBUF_INIT;\n \n \t/* search for the interesting parts */\n-\tfor (i = 0; header[i]; i++) {\n+\tfor (i = 0; i < ARRAY_SIZE(header); i++) {\n \t\tif ((!hdr_data[i] || overwrite) &&\n \t\t    parse_header(line, header[i], mi, &sb)) {\n \t\t\thandle_header(&hdr_data[i], &sb);\n@@ -625,7 +625,7 @@ static int is_inbody_header(const struct mailinfo *mi,\n {\n \tint i;\n \tconst char *val;\n-\tfor (i = 0; header[i]; i++)\n+\tfor (i = 0; i < ARRAY_SIZE(header); i++)\n \t\tif (!mi->s_hdr_data[i] && skip_header(line, header[i], &val))\n \t\t\treturn 1;\n \treturn 0;\n@@ -772,7 +772,7 @@ static int check_inbody_header(struct mailinfo *mi, const struct strbuf *line)\n \t\treturn is_format_patch_separator(line->buf + 1, line->len - 1);\n \tif (starts_with(line->buf, \"[PATCH]\") && isspace(line->buf[7])) {\n \t\tint i;\n-\t\tfor (i = 0; header[i]; i++)\n+\t\tfor (i = 0; i < ARRAY_SIZE(header); i++)\n \t\t\tif (!strcmp(\"Subject\", header[i])) {\n \t\t\t\thandle_header(&mi->s_hdr_data[i], line);\n \t\t\t\treturn 1;\n@@ -824,7 +824,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)\n \t\t * We may have already read \"secondary headers\"; purge\n \t\t * them to give ourselves a clean restart.\n \t\t */\n-\t\tfor (i = 0; header[i]; i++) {\n+\t\tfor (i = 0; i < ARRAY_SIZE(header); i++) {\n \t\t\tif (mi->s_hdr_data[i])\n \t\t\t\tstrbuf_release(mi->s_hdr_data[i]);\n \t\t\tFREE_AND_NULL(mi->s_hdr_data[i]);\n@@ -1155,7 +1155,7 @@ static void handle_info(struct mailinfo *mi)\n \tstruct strbuf *hdr;\n \tint i;\n \n-\tfor (i = 0; header[i]; i++) {\n+\tfor (i = 0; i < ARRAY_SIZE(header); i++) {\n \t\t/* only print inbody headers if we output a patch file */\n \t\tif (mi->patch_lines && mi->s_hdr_data[i])\n \t\t\thdr = mi->s_hdr_data[i];\n"},{"id":"501499","messageId":"Zsb1rGQbglHMiBHI@tanuki","threadId":"61984","inReplyTo":"xmqq1q2i6gw7.fsf@gitster.g","subject":"Re: [PATCH] mailinfo: we parse fixed headers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T08:24:12Z","receivedAt":"2024-08-22T08:24:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Aug 20, 2024 at 02:09:12PM -0700, Junio C Hamano wrote:\n> diff --git c/mailinfo.c w/mailinfo.c\n> index 94b9b0abf2..17d7c3b594 100644\n> --- c/mailinfo.c\n> +++ w/mailinfo.c\n> @@ -346,10 +346,10 @@ static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)\n>  \tstrbuf_trim(subject);\n>  }\n>  \n> -#define MAX_HDR_PARSED 10\n> -static const char *header[MAX_HDR_PARSED] = {\n> -\t\"From\",\"Subject\",\"Date\",\n> +static const char * const header[] = {\n> +\t\"From\", \"Subject\", \"Date\",\n>  };\n> +#define MAX_HDR_PARSED (ARRAY_SIZE(header) + 1)\n>  \n>  static inline int skip_header(const struct strbuf *line, const char *hdr,\n>  \t\t\t      const char **outval)\n\nI was briefly wondering whether changing MAX_HDR_PARSED is going to\ncause negative fallout. But the only place where it is used is to\nallocate both the `mi->p_hdr_data` and `mi->s_hdr_data` arrays. And\nthese are only populated via non-NULL `header`s.\n\n> @@ -583,7 +583,7 @@ static int check_header(struct mailinfo *mi,\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \n>  \t/* search for the interesting parts */\n> -\tfor (i = 0; header[i]; i++) {\n> +\tfor (i = 0; i < ARRAY_SIZE(header); i++) {\n>  \t\tif ((!hdr_data[i] || overwrite) &&\n>  \t\t    parse_header(line, header[i], mi, &sb)) {\n>  \t\t\thandle_header(&hdr_data[i], &sb);\n\nThis is also a welcome change, as it makes it way easier to spot intent\nand to notice that `header` is indeed a static array of string\nconstants.\n\nSo this change looks sensible to me, thanks!\n\nPatrick\n"}]}