Status

defect
RESOLVED FIXED
11 years ago
Last year

People

(Reporter: dougt, Assigned: dougt)

Tracking

({fixed1.9.1})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch patch v.1Splinter Review
GetProcAddress doesn't exist on windows mobile, but GetProcAddressA does.

We need to redefine GetProcAddress but it can not exist in the shunt (because windows.h is #included after us).  We will create a windows.h override file, which includes that actual sdk's window.h.  This is a pretty flexible solution and can apply to other sdk headers that we need override.
Attachment #347469 - Flags: review?(ted.mielczarek)
Comment on attachment 347469 [details] [diff] [review]
patch v.1

+WINCE_SDK      = @WINCE_SDK@

Please name this WINCE_SDK_DIR, to follow the convention set by MACOS_SDK_DIR:
http://mxr.mozilla.org/mozilla-central/source/configure.in#798

If you're going to rely on that being set, you'd better test that the variable is set, and the directory exists.

+    echo "#include \"$WINCE_SDK/windows.h\"" > $srcdir/build/wince/shunt/include/windows.h
+    echo "#define GetProcAddress GetProcAddressA" >> $srcdir/build/wince/shunt/include/windows.h

This might be clearer as a here doc, something like:

cat > $srcdir/build/wince/shunt/include/windows.h >>EOF
#include "$WINCE_SDK/windows.h"
#define GetProcAddress GetProcAddressA
EOF

r=me with those addressed.
Attachment #347469 - Flags: review?(ted.mielczarek) → review+
http://hg.mozilla.org/mozilla-central/rev/ea2328828506
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #347469 - Flags: approval1.9.1?
Comment on attachment 347469 [details] [diff] [review]
patch v.1

a191=beltzner
Attachment #347469 - Flags: approval1.9.1? → approval1.9.1+
this landed on the 1.9.1 branch.  adding keyword fixed1.9.1
Keywords: fixed1.9.1
Why were Ted's review comments not addressed before landing? His r+ was conditional on that happening.
Blocks: 475111
This fix is broken -- it essentially breaks anyone who's using GetProcAddress correctly, e.g.:

  GetProcAddress(h, TEXT("SymbolName"));

because the TEXT will expand to L"SymbolName", which isn't valid to GetProcAddressA.  Both GetProcAddressA and GetProcAddressW exist on Windows Mobile; winbase.h #defines GetProcAddress to GetProcAddressW by default.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Then again, I have no idea how the TEXT() bit works with UNICODE on the desktop, but it looks like all the relevant code is always wrapped in #ifdef WINCE.  The main problem is that this breaks the ATL includes, which have the TEXT() form with GetProcAddress.
Hrm, given all that, I think a better option for us would be to not do the #define, but to provide GetProcAddress itself, overloaded for both char* and wchar_t*, and call the right function internally..
OS: Windows XP → Windows Mobile 6 Professional
Hardware: x86 → All
I'm re-closing this bug.  GetProcAddress takes a CSTR argument, not a TSTR argument.  Calling it with TEXT("") is incorrect usage.

FARPROC WINAPI GetProcAddress(
  __in  HMODULE hModule,
  __in  LPCSTR lpProcName
);
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.