Hi,
On Thu, 14 Feb 2008, Jay Soffian wrote:
> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
> index 2600847..deaf92b 100644
> --- a/builtin-mailinfo.c
> +++ b/builtin-mailinfo.c
> @@ -20,6 +20,13 @@ static enum {
> static enum {
> TYPE_TEXT, TYPE_OTHER,
> } message_type;
> +/* RFC 3676 Text/Plain Format and DelSp Parameters */
> +static enum {
> + FORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,
> +} tp_format;
Why not call it "enum message_format"?
> +static enum {
> + DELSP_NONE, DELSP_YES, DELSP_NO,
> +} tp_delsp;
Why not call it "enum message_is_delsp"?
> @@ -193,6 +200,18 @@ static int handle_content_type(char *line)
>
> if (strcasestr(line, "text/") == NULL)
> message_type = TYPE_OTHER;
> + else if (strcasestr(line, "text/plain")) {
> + char attr[256];
> + if (slurp_attr(line, "format=", attr) && !strcasecmp(attr, "flowed")) {
> + tp_format = FORMAT_FLOWED;
> + if (slurp_attr(line, "delsp=", attr) && !strcasecmp(attr, "yes"))
> + tp_delsp = DELSP_YES;
> + else
> + tp_delsp = DELSP_NO;
> + }
> + else
> + tp_format = FORMAT_FIXED;
Does that mean that the format is only set if the content type is
"text/plain"?
> @@ -681,6 +700,8 @@ again:
> transfer_encoding = TE_DONTCARE;
> charset[0] = 0;
> message_type = TYPE_TEXT;
> + tp_format = FORMAT_NONE;
> + tp_delsp = DELSP_NONE;
>
> /* slurp in this section's info */
> while (read_one_header_line(line, sizeof(line), fin))
> @@ -770,6 +791,24 @@ static int handle_filter(char *line, unsigned linesize)
> {
> static int filter = 0;
>
> + if (tp_format == FORMAT_FLOWED && !!strcmp(line, "-- \n")) {
The !! is unnecessary; please skip it.
> + char *cp = line;
> + while (*cp == '>' && *cp != 0)
> + cp++;
> + if (*cp == ' ')
> + cp++;
> + line = cp;
How about using strchrnul()?
Note: I do not know enough about format=flawed to know why you should skip
to "> ". I would have expected "\n", though.
> + if (!!strcmp(line, "-- \n")) {
The !! is unnecessary; please skip it.
> + while (*cp != '\n' && *cp !=0)
> + cp++;
Again, this is the job for strchrnul().
> + if (cp > line && *cp == '\n' && *(cp-1) == ' ') {
> + if (tp_delsp == DELSP_YES)
> + *(cp-1) = '\0';
> + else
> + *cp = '\0';
> + }
Or maybe
cp[0 - (tp_delsp == DELSP_YES)] = '\0';
But maybe that is too cute.
But another thing struck me here: why setting *cp = '\0'; only if *(cp-1)
== ' ', even if tp_delsp != DELSP_YES?
> @@ -818,6 +857,7 @@ static void handle_body(void)
>
> switch (transfer_encoding) {
> case TE_BASE64:
> + case TE_QP:
> {
> char *op = line;
Did that just slip in, or was this intended. If the latter, is this
related to format=flawed, or is it a bug fix in its own right?
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
