On Mon, 17 May 2004 22:46:22 -0400
Sam Varshavchik <[EMAIL PROTECTED]> wrote:

> I've reviewed your patch in greater detail and I have a few more things that 
> need to be cleared up.
> 
> But first, I've updated your patch against sqwebmail-4.0.4 because there 
> were a number of structural source changes that conflicted with your patch, 
> and it's faster for me to figure out what had to be done to reapply the 
> patch against the 4.0.4 codebase.  So, working from the following patch:
> 
> 1) In rfc822/rfc2047.c, your original patch added "*str <= 0x20" and "str[j] 
> >= 0x20".
> I believe that in both instances the char value must be casted to (unsigned
> char), for portability reasons.

I see.


> 2) I believe that both UTF-8 and UTF-7 character sets should have the 
> UNICODE_WORD_WRAPPABLE flag set.

UNICODE_WORD_WRAPPABLE depends on both language and script.
Domestic charsets almostly correspond to language-script, but on
UTF context they may not be determined.


> 3) There's a lot of duplicated code in rfc822/encode.c (previous 
> rfc2045/rfc2045encode.c).  You've copied a large function only to change 
> two/three lines in the middle of it.  This is sloppy, and unmaintainable.

I'll try to wring it out.


> 4) Originally your patch read for what's now rfc822/encode.c:
> 
> +     if (start_pos >= 0)
> +             if ((pos=fseek(fp, start_pos, SEEK_SET)) == (off_t)-1)
> +                     return NULL;
>  
> This is wrong.  fseek() returns 0 upon success, not start_pos, and given the 
> following code it is clear that pos should be set to start_pos in this case.

Indeed.  I might read the manpages over again.


> I have updated your patch to include all of the above changes except for #3. 
> Please apply it to 4.0.4, and test.

Thank you for detailed review.  I'll update and test my patch 
by this weekend.


  --- nezumi

Reply via email to