reduce narrow windows API calls in imagelib

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: blassey, Assigned: crowderbt)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Attachment #309225 - Flags: review?(pavlov)
Blocks: 418703
(Assignee)

Comment 1

11 years ago
Review ping?

Comment 2

11 years ago
you have:

+      PRUnichar specialNativePath[MAX_PATH];
+      ::SHGetPathFromIDListW(idList, specialNativePath);
+      ::GetShortPathNameW(specialNativePath, specialNativePath, sizeof(specialNativePath));

Please see:
http://msdn.microsoft.com/en-us/library/aa364989(VS.85).aspx

I think that the patch should use sizeof(specialNativePath)/sizeof(specialNativePath[0]) to have the count of chars (cch).

Please note that NS_ARRAY_LENGTH could be used here and in other places. It is defined here:
http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsMemory.h
(Assignee)

Comment 3

11 years ago
Posted patch v2Splinter Review
Updates some buffer issues (as Bernard noticed) and brings in line with trunk.
Assignee: nobody → crowder
Attachment #309225 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #334779 - Flags: review?(pavlov)
Attachment #309225 - Flags: review?(pavlov)

Comment 4

11 years ago
(In reply to comment #3)
> Created an attachment (id=334779) [details]
> v2

Looks good.

While you're touching that file, you might consider fixing bug 284156 along with it. You'd just have to do:

-      IMalloc* pMalloc;
-      hr = ::SHGetMalloc(&pMalloc);
-      if (SUCCEEDED(hr)) {
-        pMalloc->Free(idList);
-        pMalloc->Release();
-      }
+      ::CoTaskMemFree(idList);

This is very safe, as SHGetMalloc is deprecated per MSDN.


Updated

11 years ago
Attachment #334779 - Flags: review?(pavlov) → review+
(Assignee)

Comment 5

11 years ago
http://hg.mozilla.org/mozilla-central/rev/e17e0d80a962
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Can patch for this bug cause Bug 454247?
(Assignee)

Comment 7

10 years ago
Certainly possible, would you care to try rolling it back?
(In reply to comment #7)
> Certainly possible, would you care to try rolling it back?
Unfortunately I don't have windows build environment, I'm using linux.
You need to log in before you can comment on or make changes to this bug.