Re: [PATCH 06/26] mailinfo: always pass "line" as an argument

2015-10-14 Thread Stefan Beller 
On Tue, Oct 13, 2015 at 4:16 PM, Junio C Hamano  wrote:
> Some functions in this module accessed the global "struct strbuf
> line" while many others used a strbuf passed as an argument.
> Convert the former to ensure that nobody deeper in the callchains
> relies on the global one.
>
> Signed-off-by: Junio C Hamano 
> ---
>  builtin/mailinfo.c | 48 
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 5a74811..c3c7d67 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -12,7 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
>  static int keep_subject;
>  static int keep_non_patch_brackets_in_subject;
>  static const char *metainfo_charset;
> -static struct strbuf line = STRBUF_INIT;
> +static struct strbuf line_global = STRBUF_INIT;
>  static struct strbuf name = STRBUF_INIT;
>  static struct strbuf email = STRBUF_INIT;
>  static char *message_id;
> @@ -817,23 +817,23 @@ static void handle_filter(struct strbuf *line, int 
> *filter_stage, int *header_st
> }
>  }
>
> -static int find_boundary(void)
> +static int find_boundary(struct strbuf *line)
>  {
> -   while (!strbuf_getline(, fin, '\n')) {
> -   if (*content_top && is_multipart_boundary())
> +   while (!strbuf_getline(line, fin, '\n')) {
> +   if (*content_top && is_multipart_boundary(line))
> return 1;
> }
> return 0;
>  }
>
> -static int handle_boundary(int *filter_stage, int *header_stage)
> +static int handle_boundary(struct strbuf *line, int *filter_stage, int 
> *header_stage)
>  {
> struct strbuf newline = STRBUF_INIT;
>
> strbuf_addch(, '\n');
>  again:
> -   if (line.len >= (*content_top)->len + 2 &&
> -   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
> +   if (line->len >= (*content_top)->len + 2 &&
> +   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
> /* we hit an end boundary */
> /* pop the current boundary off the stack */
> strbuf_release(*content_top);
> @@ -852,7 +852,7 @@ again:
> strbuf_release();
>
> /* skip to the next boundary */
> -   if (!find_boundary())
> +   if (!find_boundary(line))
> return 0;
> goto again;
> }
> @@ -862,18 +862,18 @@ again:
> strbuf_reset();
>
> /* slurp in this section's info */
> -   while (read_one_header_line(, fin))
> -   check_header(, p_hdr_data, 0);
> +   while (read_one_header_line(line, fin))
> +   check_header(line, p_hdr_data, 0);
>
> strbuf_release();
> /* replenish line */
> -   if (strbuf_getline(, fin, '\n'))
> +   if (strbuf_getline(line, fin, '\n'))
> return 0;
> -   strbuf_addch(, '\n');
> +   strbuf_addch(line, '\n');
> return 1;
>  }
>
> -static void handle_body(void)
> +static void handle_body(struct strbuf *line)
>  {
> struct strbuf prev = STRBUF_INIT;
> int filter_stage = 0;
> @@ -881,24 +881,24 @@ static void handle_body(void)
>
> /* Skip up to the first boundary */
> if (*content_top) {
> -   if (!find_boundary())
> +   if (!find_boundary(line))
> goto handle_body_out;
> }
>
> do {
> /* process any boundary lines */
> -   if (*content_top && is_multipart_boundary()) {
> +   if (*content_top && is_multipart_boundary(line)) {
> /* flush any leftover */
> if (prev.len) {
> handle_filter(, _stage, 
> _stage);
> strbuf_reset();
> }
> -   if (!handle_boundary(_stage, _stage))
> +   if (!handle_boundary(line, _stage, 
> _stage))
> goto handle_body_out;
> }
>
> /* Unwrap transfer encoding */
> -   decode_transfer_encoding();
> +   decode_transfer_encoding(line);
>
> switch (transfer_encoding) {
> case TE_BASE64:
> @@ -907,7 +907,7 @@ static void handle_body(void)
> struct strbuf **lines, **it, *sb;
>
> /* Prepend any previous partial lines */
> -   strbuf_insert(, 0, prev.buf, prev.len);
> +   strbuf_insert(line, 0, prev.buf, prev.len);
> strbuf_reset();
>
> /*
> @@ -915,7 +915,7 @@ static void handle_body(void)
>  * multiple new lines.  Pass only one chunk
>  * at a time to handle_filter()
>  */
> -

Re: [PATCH 06/26] mailinfo: always pass "line" as an argument

2015-10-14 Thread Junio C Hamano 
Stefan Beller  writes:

>> @@ -1019,10 +1019,10 @@ static int mailinfo(FILE *in, FILE *out, const char 
>> *msg, const char *patch)
>> ungetc(peek, in);
>>
>> /* process the email header */
>> -   while (read_one_header_line(, fin))
>> -   check_header(, p_hdr_data, 1);
>> +   while (read_one_header_line(_global, fin))
>> +   check_header(_global, p_hdr_data, 1);
>
> This is the only function to use line_global if I see correctly.
> The function is called only once, so no need to preserve state
> outside the function. Would it make sense to remove line_global
> completely and have a local variable in this function instead?

That is exactly the step that comes after it does, but if you squash
06 and 07 into one patch (i.e. take diff between the state after 05
and after 07), that realization will not easily come (well, at least
it didn't come to me and I wasn't convinced that the conversion is
correct myself until I split 06 and 07 into two separate steps).


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/26] mailinfo: always pass "line" as an argument

2015-10-13 Thread Junio C Hamano 
Some functions in this module accessed the global "struct strbuf
line" while many others used a strbuf passed as an argument.
Convert the former to ensure that nobody deeper in the callchains
relies on the global one.

Signed-off-by: Junio C Hamano 
---
 builtin/mailinfo.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 5a74811..c3c7d67 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,7 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
-static struct strbuf line = STRBUF_INIT;
+static struct strbuf line_global = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
 static char *message_id;
@@ -817,23 +817,23 @@ static void handle_filter(struct strbuf *line, int 
*filter_stage, int *header_st
}
 }
 
-static int find_boundary(void)
+static int find_boundary(struct strbuf *line)
 {
-   while (!strbuf_getline(, fin, '\n')) {
-   if (*content_top && is_multipart_boundary())
+   while (!strbuf_getline(line, fin, '\n')) {
+   if (*content_top && is_multipart_boundary(line))
return 1;
}
return 0;
 }
 
-static int handle_boundary(int *filter_stage, int *header_stage)
+static int handle_boundary(struct strbuf *line, int *filter_stage, int 
*header_stage)
 {
struct strbuf newline = STRBUF_INIT;
 
strbuf_addch(, '\n');
 again:
-   if (line.len >= (*content_top)->len + 2 &&
-   !memcmp(line.buf + (*content_top)->len, "--", 2)) {
+   if (line->len >= (*content_top)->len + 2 &&
+   !memcmp(line->buf + (*content_top)->len, "--", 2)) {
/* we hit an end boundary */
/* pop the current boundary off the stack */
strbuf_release(*content_top);
@@ -852,7 +852,7 @@ again:
strbuf_release();
 
/* skip to the next boundary */
-   if (!find_boundary())
+   if (!find_boundary(line))
return 0;
goto again;
}
@@ -862,18 +862,18 @@ again:
strbuf_reset();
 
/* slurp in this section's info */
-   while (read_one_header_line(, fin))
-   check_header(, p_hdr_data, 0);
+   while (read_one_header_line(line, fin))
+   check_header(line, p_hdr_data, 0);
 
strbuf_release();
/* replenish line */
-   if (strbuf_getline(, fin, '\n'))
+   if (strbuf_getline(line, fin, '\n'))
return 0;
-   strbuf_addch(, '\n');
+   strbuf_addch(line, '\n');
return 1;
 }
 
-static void handle_body(void)
+static void handle_body(struct strbuf *line)
 {
struct strbuf prev = STRBUF_INIT;
int filter_stage = 0;
@@ -881,24 +881,24 @@ static void handle_body(void)
 
/* Skip up to the first boundary */
if (*content_top) {
-   if (!find_boundary())
+   if (!find_boundary(line))
goto handle_body_out;
}
 
do {
/* process any boundary lines */
-   if (*content_top && is_multipart_boundary()) {
+   if (*content_top && is_multipart_boundary(line)) {
/* flush any leftover */
if (prev.len) {
handle_filter(, _stage, 
_stage);
strbuf_reset();
}
-   if (!handle_boundary(_stage, _stage))
+   if (!handle_boundary(line, _stage, 
_stage))
goto handle_body_out;
}
 
/* Unwrap transfer encoding */
-   decode_transfer_encoding();
+   decode_transfer_encoding(line);
 
switch (transfer_encoding) {
case TE_BASE64:
@@ -907,7 +907,7 @@ static void handle_body(void)
struct strbuf **lines, **it, *sb;
 
/* Prepend any previous partial lines */
-   strbuf_insert(, 0, prev.buf, prev.len);
+   strbuf_insert(line, 0, prev.buf, prev.len);
strbuf_reset();
 
/*
@@ -915,7 +915,7 @@ static void handle_body(void)
 * multiple new lines.  Pass only one chunk
 * at a time to handle_filter()
 */
-   lines = strbuf_split(, '\n');
+   lines = strbuf_split(line, '\n');
for (it = lines; (sb = *it); it++) {
if (*(it + 1) == NULL) /* The last line */
if (sb->buf[sb->len - 1] != '\n') {