248768 - parser code for IMAP and addrbook: stroken_r: pull end-of-string checks out of char-delimiter loop

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Hans-Andreas Engel]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040622
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040622

In stroken_r(), a string is tokenized until the end of this string is reached.  
Currently, the check for this end-of-string condition is carried out somewhat ne
sted within the loop over the list of token delimiters.  This check should be p
ulled out of the loop to increase readability of the code.  Performance can be s
lightly increased.

stroken_r is defined in
  mailnews/imap/src/nsIMAPGenericParser.cpp
  mailnews/addrbook/src/nsDirPrefs.cpp

Patch forthcoming.

Reproducible: Always
Steps to Reproduce:

Comment 1

[Hans-Andreas Engel]

Attachment: proposed patch

Refactored stroken_r, behavior of stroken_r remains unchanged.

Note that this patch should not impair performance.  With the new code and for
each input, the number of branches is never larger than in the original code.

Comment 2

[David :Bienvenu]

Comment on attachment 151785 [details] [diff] [review]
proposed patch

looks OK - I'm going to try running with the patch. Also, I'm going to fix the
braces to match the rest of the imap code, and remove the tabs...

Comment 3

[Hans-Andreas Engel]

Thank you for your review!  I only removed the tabs from the lines which I have
changed such that the patch remains short and clear.  Cleaning up the tabs now
sounds good.

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Punting bug into confirmed state, since David is looking at the patch...

Comment 5

[[e-mail]]

Comment on attachment 151785 [details] [diff] [review]
proposed patch

Although I would have thought this could have been further optimized since the
call sites are virtually identical.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

filed Bug 253794 on unifying these functions, and possibly replacing them with
nsCRT::strtok_r

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Fix landed by timeless:

07/31/2004 21:02

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using LXR for code inspection:

1.43	neil%parkwaycc.co.uk	2004-12-07 16:02	 	Bug 273080 Remove double assignment
fNextToken = nsIMAPGenericParser::GetNextToken() p=[e-mail]
r=bienvenu sr=dmose