| Message ID | 35ef5149-2da9-a147-fb5b-8f175ff617e7@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] strbuf: add and use strbuf_insertstr() | expand |
René Scharfe <l.s.r@web.de> writes: > handle_content_type() only cares about the value after "Content-Type: "; > there is no need to insert that string for it. > > Suggested-by: Eric Sunshine <sunshine@sunshineco.com> > Signed-off-by: René Scharfe <l.s.r@web.de> > --- > mailinfo.c | 1 - > 1 file changed, 1 deletion(-) OK. There is only one caller that just saw that the line it is looking at begins with "Content-Type:", and the only thing the helper function does is to look for a handful of attr=value pairs (without even looking at the primary thing the header wants to convey---the type itself, like text/plain etc.). The helper may want to learn a bit more careful than the current implementation in doing what it does, but it does not need to see the header name to do so. It's not like we will be accepting more than one kind of content-type like header with this helper and make it capable of tailoring its behaviour based on which exact header type it is. Will queue. Thanks. > diff --git a/mailinfo.c b/mailinfo.c > index 543962d40c..402ef04dd1 100644 > --- a/mailinfo.c > +++ b/mailinfo.c > @@ -570,7 +570,6 @@ static int check_header(struct mailinfo *mi, > len = strlen("Content-Type: "); > strbuf_add(&sb, line->buf + len, line->len - len); > decode_header(mi, &sb); > - strbuf_insertstr(&sb, 0, "Content-Type: "); > handle_content_type(mi, &sb); > ret = 1; > goto check_header_out; > -- > 2.25.0
Hi René, On Mon, Feb 10, 2020 at 08:15:19AM +0100, René Scharfe wrote: > handle_content_type() only cares about the value after "Content-Type: "; > there is no need to insert that string for it. > > Suggested-by: Eric Sunshine <sunshine@sunshineco.com> > Signed-off-by: René Scharfe <l.s.r@web.de> > --- > mailinfo.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/mailinfo.c b/mailinfo.c > index 543962d40c..402ef04dd1 100644 > --- a/mailinfo.c > +++ b/mailinfo.c > @@ -570,7 +570,6 @@ static int check_header(struct mailinfo *mi, > len = strlen("Content-Type: "); > strbuf_add(&sb, line->buf + len, line->len - len); > decode_header(mi, &sb); > - strbuf_insertstr(&sb, 0, "Content-Type: "); This looks quite good to me. When I first looked, I was wondering if we should have used 'skip_prefix' and passed that result into 'strbuf_insertstr', but I think that's a bigger change than we need for now. This looks all right to me. > handle_content_type(mi, &sb); > ret = 1; > goto check_header_out; > -- > 2.25.0 Here is my: Reviewed-by: Taylor Blau <me@ttaylorr.com> Thanks, Taylor
diff --git a/mailinfo.c b/mailinfo.c index 543962d40c..402ef04dd1 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -570,7 +570,6 @@ static int check_header(struct mailinfo *mi, len = strlen("Content-Type: "); strbuf_add(&sb, line->buf + len, line->len - len); decode_header(mi, &sb); - strbuf_insertstr(&sb, 0, "Content-Type: "); handle_content_type(mi, &sb); ret = 1; goto check_header_out;
handle_content_type() only cares about the value after "Content-Type: "; there is no need to insert that string for it. Suggested-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: René Scharfe <l.s.r@web.de> --- mailinfo.c | 1 - 1 file changed, 1 deletion(-) -- 2.25.0