bugThe nmh Mail Handling System - Bugs: bug #3200, fmt_compile incorrectly optimizes...

 
 

bug #3200: fmt_compile incorrectly optimizes address parsing

Submitted by:  Jeffrey C Honig <jchonig>
Submitted on:  Sun 13 Apr 2003 02:41:59 AM UTC  
 
Category: user interfaceSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: NoneOpen/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Tue 30 Sep 2003 02:40:54 PM UTC, comment #3:

Fixed by jchonig on 11 Aug 2003, fmt_scan.c rev 1.8

Glenn Burkhardt <gbburkhardt>
Project Member
Wed 23 Jul 2003 05:48:54 PM UTC, comment #2:

Here is an updated description of this problem. A new and improved diff is attached.

When compiling format strings, nmh attempts to avoid multiple parsing of
address and date fields by only inserting calls to the parse functions
(FT_PARSEADDR and FT_PARSEDATE) for a given component once. The problem
with this method is that the initial invocation may actually be on a
code path that is conditionally executed. This can result cached
copies of data from the fields in previous messages to be used.

My solution is to move this optimization from compile time to run time.
Address and Date parsing calls (FT_PARSEADDR and FT_PARSEDATE) will
always be included. Run time flags are used to prevent these functions
from being run more than once per component per message.

The c_flags field has being converted from a boolean to a bit-field to
facilitate maintenance of the new CT_ADDRPARSED and CT_DATEPARSED
values. The result value that used to be in this field is now the bit
CF_TRUE and the overloaded use of this field by scan() is now the
CT_DATEFAB bit.

Some unneeded flags (CT_ADDRPARSE, CT_MYMBOX) have also been removed.

Jeffrey C Honig <jchonig>
Project Member
Sun 13 Apr 2003 02:48:03 AM UTC, comment #1:

Here is the fix I promised.

Instead of testing CT_ADDRFIELD in do_name() in fmt_compile.c, always compile in the FT_PARSEADDR function. This insures that the address is parsed in all cases before it is used.

In order to avoid duplicate parsing, we need a flag to indicate that the FT_PARSEADDR case has been visited. Since c_flags seems to have been intended to be flags, I define bits for it.

CF_TRUE is a truth value, mostly used to determine if the component is present.

CF_DATEFAB means that a Date: header was fabricated, see the code in scansbr.c.

CF_ADDRPARSED means that the FT_PARSEADDR case has already been visited on this component.

Jeffrey C Honig <jchonig>
Project Member
Sun 13 Apr 2003 02:41:59 AM UTC, original submission:

The code in fmt_compile assumes that if the mymbox function is referenced that the internal parseaddr functionality has been called as the field has been referenced. This is not the case with the attached scan format (a simplied version of what we use in MH-E).

Also see MH-E bug 686398 <http://sourceforge.net/tracker/index.php?func=detail&aid=686398&group_id=13357&atid=113357>.

A patch will follow.

Below is an excerpt from the MH-E bug:

Either mh-scan-format-nmh is buggy or their is a bug in
nmh scan (this is the bug that Kevin Layer found). To
see the problem the following steps are needed:

(1) Move the mail file 35 to a folder which has at
least one other mail before it. Edit it to replace the
From: header field with your address.

(2) Use scan.mhe (this is our scan-format written to a
file) to run scan as follows:
scan -form scan.mhe +test
You will see that the address field of the scan line
for 35 is wrong and gets its value from the previous mail.

I have minimized the scan format file to create
scan.mhe-minimal which still shows the problem. I could
find two ways of working around the problem but both
are sub-optimal:

(1) Remove the "friendly" and just use decode (edit
line 10 of scan.mhe). This is bad since it changes the
way the scan line looks.

(2) Remove the "t" annotation to designate the mails to
me (remove line 4 of scan.mhe).

Jeffrey C Honig <jchonig>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #553:  diff added by jchonig (8KiB - application/octet-stream - Updated patch)
file #498:  diff. added by jchonig (6KiB - application/octet-stream - Proposed patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

Please enter the title of George Orwell's famous dystopian book (it's a date):

 

 

Follow 2 latest changes.

Date Changed By Updated Field Previous Value => Replaced By
Tue 30 Sep 2003 02:40:54 PM UTCgbburkhardtStatusNone=>(Error - Not Found)
  Open/ClosedOpen=>(Error - Not Found)

Back to the top


Powered by Savane 3.1-cleanup1