Last Comment Bug 312569 - nsIVersionComparator is still broken in some circumstances
: nsIVersionComparator is still broken in some circumstances
: fixed1.8
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: x86 Windows XP
P1 normal (vote)
: ---
Assigned To: Benjamin Smedberg [:bsmedberg]
: Nathan Froyd [:froydnj]
Depends on:
  Show dependency treegraph
Reported: 2005-10-15 11:44 PDT by Nickolay_Ponomarev
Modified: 2006-03-12 18:59 PST (History)
2 users (show)
benjamin: blocking1.8rc1+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Always initialize versionparts, rev. 1 (1.87 KB, patch)
2005-10-17 12:04 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: review+
mscott: approval1.8rc1+
Details | Diff | Splinter Review

Description User image Nickolay_Ponomarev 2005-10-15 11:44:05 PDT
The following code returns 1 instead of 0 for me.

          .compare("0+", "1pre");

This happens because ParseVP() doesn't initialize numC and extraD if strB[0]=='+'. 

Also, either check the other two parts or document that you ignore them in case
if strB starts with '+' (in the nsIVersionComparator.idl). Currently the
documentation implies that the other two parts are evaluated even if strB is '+'.
Comment 1 User image Nickolay_Ponomarev 2005-10-15 11:45:07 PDT
For some reason, on the second iteration of the loop in NS_CompareVersions numC
and extraD get nulled, and 1.0+ correctly compares as equal to 1.1pre, but I
think this may break on certain compilers. Asking for blocking1.8 for that reason.

Don't have a tree, so can't make a patch, sorry.
Comment 2 User image Scott MacGregor 2005-10-17 10:57:45 PDT
benjamin, do you think this is a real problem that we should stop the 1.5
release for?
Comment 3 User image Benjamin Smedberg [:bsmedberg] 2005-10-17 11:39:07 PDT
Yeah, there seem to be cases where uninitialized memory can peek through.
Comment 4 User image Benjamin Smedberg [:bsmedberg] 2005-10-17 12:04:45 PDT
Created attachment 199828 [details] [diff] [review]
Always initialize versionparts, rev. 1

This initializes the versionparts up front, which is probably how I should have
done it in the first place.
Comment 5 User image Benjamin Smedberg [:bsmedberg] 2005-10-17 13:32:52 PDT
Fixed on trunk.
Comment 6 User image Benjamin Smedberg [:bsmedberg] 2005-10-18 08:43:20 PDT
Fixed on 1.8 branch.

Note You need to log in before you can comment on or make changes to this bug.