Re: [evolution-patches] patch for fix fix non rfc2047 compliant i18n mailer

[Jeffrey Stedfast <fejj ximian com>]

On Sat, 2004-05-29 at 18:13, cantona wrote:
> Hi all, 
> 
> 	This is the patch for fix non rfc2047 compliant i18n header. 
> I have tested it with no problem and work properly with valid header.
> If someone have time, please see it if something I can improve . (I am a
> newbie coder)
> 
> Thanks,
> Cantona 

> Index: camel/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/camel/ChangeLog,v
> retrieving revision 1.2152
> diff -u -r1.2152 ChangeLog
> --- a/camel/ChangeLog   27 May 2004 17:56:51 -0000      1.2152
> +++ b/camel/ChangeLog   29 May 2004 22:04:56 -0000
> @@ -1,3 +1,10 @@
> +2004-05-30  Cantona Su  <paradisetux hotmail com>
> +                                                                               
> +       * camel-mime-utils.c (camel_fix_non_rfc): Added
> camel_fix_non_rfc()
> +       for fix non rfc2047 compliant i18n mailer. #58555
> +       (camel_header_decode_string): call camel_fix_non_rfc() before
> return
> +       the string to header_decode_text().
> +                                                                                                                          
>  2004-05-27  Jeffrey Stedfast  <fejj novell com>
>  
>         Fixes bug #59191.
> Index: camel/camel-mime-utils.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/camel/camel-mime-utils.c,v
> retrieving revision 1.208
> diff -u -r1.208 camel-mime-utils.c
> --- a/camel/camel-mime-utils.c  24 May 2004 08:02:10 -0000      1.208
> +++ b/camel/camel-mime-utils.c  29 May 2004 22:04:58 -0000
> @@ -70,6 +70,7 @@
>  #define w(x)
>  
>  #define d(x)
> +#define dd(x)
>  #define d2(x)

would be better to remove dd() once you get a working patch to make it
so that the patch doesn't alter as many lines without need.

[snip]

> @@ -1174,7 +1175,51 @@
>  {
>         if (in == NULL)
>                 return NULL;
> -       return header_decode_text (in, strlen (in), default_charset);
> +       const char *enc;

you can't declare a variable here. you MUST declare it at the top of the
function in order to comply with ansi-c/c89

> +       enc = camel_fix_non_rfc(in);

you have not yet declared camel_fix_non_rfc() so this will produce a
warning. camel_fix_non_rfc() should be a provate function to this file
(eg. use the keyword 'static'). Also, this is a bad name... should
probably be more like fix_broken_rfc2047() or something to that effect.

> +       return header_decode_text (enc, strlen (enc),
> default_charset);

oops. you leaked 'enc'

> +}
> +
> +/* fix for non rfc2047 compliant i18n mailers */
> +const char *
> +camel_fix_non_rfc(const char *in) {

1. this should not return const char *, because that's not what it
actually returns :-)

2. put the brace on its own line like the rest of the functions in the
file.

> +        GString *out;
> +       const char *str, *start, *temp;
> +        int len, i;

please don't keep changing your indent. just use a single tab.

> +        
> +       str = in;
> +        out = g_string_new ("");
> +       
> +       do {
> +                start = strstr(str, "=?");
> +                       temp = strstr(toupper(str), "?B?"); 

this isn't doing what you think it does. read the man page for toupper()
carefully :-)

also, what about "?Q?" ? :-)

> +               
> +               if (start == str && (start = strstr(str+2, "=?")) &&
> temp) {
> +                       len = start - str + 3;
> +               } else {
> +                       len = start - str;
> +               }

what is this supposed to be doing?

what if initially start == str but the strstr then fails?

> +               
> +               dd(printf("encode word %.*s\n\n", len, str));
> +               g_string_append_len(out, str, len);
> +               g_string_append_c(out, ' ');
> +               
> +               temp = out->str;
> +               i = strrchr(temp, '=') - temp + 1;

what if strrchr() returns NULL here?

> +               
> +               if(i > 0 && temp[i-2] == '?' &&
> !camel_mime_is_lwsp(temp[i]))
> +                       g_string_insert(out, i, ' ');
> +                       
> +               if(start >= str) 
> +                        str += len;
> +               
> +               temp = NULL;       
> +        } while (start != NULL);
> +
> +        dd(printf("encode string %s\n\n", str));
> +       str = out->str;
> +       g_string_free(out, FALSE);
> +       return (const char *)str;
>  }

Jeff