nsNameValuePairDB::RenameTmp() needs to be static

RESOLVED WONTFIX

Status

Core Graveyard
GFX
P3
normal
RESOLVED WONTFIX
16 years ago
3 years ago

People

(Reporter: Jon Smirl, Assigned: Kevin McCluskey (gone))

Tracking

(Blocks: 1 bug)

Trunk
Future
x86
Windows 2000
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
The design of nsNameValuePairDB holds the file open until the object is 
destroyed. This causes RenameTmp() to try to rename a file while it is open. 
Renaming open files is illegal on Windows. Making this static allows the 
nsNameValuePairDB object to be destroyed, and then the file file can be renamed.

Comment 1

16 years ago
wrong component
Assignee: pavlov → kmcclusk
Component: Image: GFX → GFX Compositor
QA Contact: tpreston → petersen
(Reporter)

Comment 2

16 years ago
Created attachment 86839 [details] [diff] [review]
makes it static and exports symbols
(Reporter)

Updated

16 years ago
Blocks: 149163

Comment 3

16 years ago
-  if (!mAtEndOfGroup || mError)
-    goto Rename_Error;

Do you know that it is safe to remove this test?

-class nsNameValuePairDB {
+class NS_GFX nsNameValuePairDB {
...
-class nsRegion
+class NS_GFX nsRegion

I have no knowledge of what this might mean so I cannot review these. You will 
need to ask someone else to review these bits.
(Reporter)

Comment 4

16 years ago
The NS_GFX causes the symbols to be exported on Windows. Does nothing on other 
platforms.

if (!mAtEndOfGroup || mError)
-    goto Rename_Error;

This is checking to make sure that you don't rename the file while it is open 
and in an invalid state. I'm not even sure if all Unixes work the same when an 
open file is renamed.

The static functions allows you to close the file and then rename it. If the 
file is closed mAtEndOfGroup, mError don't apply.

Comment 5

16 years ago
-  if (!mAtEndOfGroup || mError)
-    goto Rename_Error;

Could you move this test to the caller? If you feel the test is not necessary 
under any possible condition then add asserts in the callers.
(Reporter)

Comment 6

16 years ago
By deleting or letting nsNameValuePairDB object go out of context, the file 
gets closed. After the nsNameValuePairDB object is gone, the variables are gone 
too. If rename is static it doesn't have any member variables.  I guess you 
could move this test to the destructor.

Comment 7

16 years ago
How moving it to the caller?

Does the Linux version compile right with this patch?

If it does not compile you should say this so that reviewers do not spend
time on something that could not be checked in.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

16 years ago
Blocks: 123569
(Assignee)

Updated

16 years ago
Priority: -- → P3
Target Milestone: --- → Future
Product: Core → Core Graveyard
This bug has been buried in the graveyard and has not been updated in over 5 years. It is probably safe to assume that it will never be fixed, so resolving as WONTFIX.

[Mass-change filter: graveyard-wontfix-2014-09-24]
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.