Closed Bug 49866 Opened 20 years ago Closed 20 years ago
Special System Directory .cpp UMR & Misuse of Get Environment Variable .
The change to -r1.31 of nsSpecialSystemDirectory.cpp under line 587 is a little broken. http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsSpecialSystemDirectory.cpp#58 7 This appears to cause a UMR in Purify. GetEnvironmentVariable is documented to return 0 if the var was not found in the environment. This leaves you calling PL_strlen on an uninitialied buffer. It looks like you cloned or modified mcmullen's code. I don't know why it is using ">= 0". It looks like '> 0' is appropriate since it looks like you only want it to go this way if "HOME" is set. It is not clear to me if the case below that where it checks for "HOMEDRIVE" ought to be '>= 0' or '> 0'. If '>= 0' *is* appropriate, then you should do "path = 0;" before the call to GetEnvironmentVariable to ensure that the buffer is safely zero terminated first. Note that release builds will leave garbage in auto variables, while debug builds generally initialize them to 0.
This is a *very* likely cause of bug 48110 which keeps *many* people from using the release build of the product. Having a bit of code like this that will work consistently in debug builds and be unpredicatable in release builds is *very* dangerous. I suggest that one of you people who wrote, reviewed, or checked in this faulty code DROP EVERYTHING RIGHT NOW and just fix this broken code.
Putting on [dogfood+] radar.
I will fix it today.
Status: NEW → ASSIGNED
jband is correct. >= is evil. When I added the check for the HOME directory, I just copied and pasted the checks for HOMEDIR/HOMEPATH. I should have review the calls. The fix has been reviewed and tested. I will check it in when the tree opens.
For what it's worth, as the reporter of bug 48110, I'd like to point out that "set home=c:\windows\temp" (just to get a HOME directory) does not fix 48110; it still crashes even with a HOME directory. Of course, it's entirely possible that it's an additional consequence of this problem (without the simple remedy of setting HOME), but I figured I'd bring it to your attention (since none of you are on 48110's CC: list).
hmm. if you sent HOME do something and still crash, my bet is that it is happening somewhere else. With any luck, tomorrow will include a fix to this area.
Fix checked in...marking as such.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Hey Doug, I'd like to verify this bug. Have you tested your fix out and is it ok. Thanks.
Marking Verified. Please reopen if problem reoccurs.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.