Closed
Bug 726576
Opened 13 years ago
Closed 13 years ago
Optimize calls to nsLocalFileWin's HasFileAttribute
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: bbondy, Assigned: bbondy)
Details
Attachments
(1 file, 1 obsolete file)
2.13 KB,
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
HasFileAttribute does 2 calls to get the file attributes, one from ResolveAndStat and the other from GetFileAttributesW right after it.
This can be optimized to do only one by using Resolve instead.
Also this function doesn't check for INVALID_FILE_ATTRIBUTES so if ResolveAndStat was not dirty, and the state changed so the file got deleted, this function would return incorrect results before.
Also the code in IsFile and IsDirectory can be simplified as Neil suggested previously to use HasFileAttribute instead.
Attachment #596670 -
Flags: review?(neil)
Comment 1•13 years ago
|
||
Comment on attachment 596670 [details] [diff] [review]
Patch v1.
>- if (INVALID_FILE_ATTRIBUTES == attributes) {
Bah, I hadn't noticed that you like your comparisons the wrong way around ;-)
>+ DWORD attributes = GetFileAttributesW(mResolvedPath.get());
::GetFileAttributesW ?
Attachment #596670 -
Flags: review?(neil) → review+
Assignee | ||
Comment 2•13 years ago
|
||
> Bah, I hadn't noticed that you like your comparisons the wrong way around ;-)
I'm pretty indifferent about it but I've been asked to switch it after some review comments so I just start off that way usually now :)
> ::GetFileAttributesW ?
I think we're moving away from ::, but if you want I can do it.
Assignee | ||
Comment 3•13 years ago
|
||
and by we I mean me :)
Assignee | ||
Comment 4•13 years ago
|
||
Returned converted windows error code instead of always returning invalid path since it was causing try failures. Carrying forward r+.
Attachment #596670 -
Attachment is obsolete: true
Attachment #598221 -
Flags: review+
Assignee | ||
Comment 5•13 years ago
|
||
Comment 6•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•