Closed Bug 971822 Opened 6 years ago Closed 6 years ago

Fix string_util.h combined with strsafe.h on mingw.

Categories

(Core :: IPC, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch fix (obsolete) — Splinter Review
The problem is visible in TaskbarPreviewButton.cpp compilation on mingw. That file uses strsafe.h, which is PSDK header that provides a few "safe" versions of string APIs. It also deprecates some of standard string APIs. This deprecations have two forms:
- if #pragma deprecate is supported, it is used
- otherwise use something like #define swprintf swprintf_instead_use_StringCbPrintfW_or_StringCchPrintfW;
Since GCC doesn't support this pragma, the define is used. That's problematic for string_util.h, which has its own versions of snprintf and swprintf inside a namespace. The mentioned define makes it fail. The attached patch simply #undefs required identifiers.
Attachment #8374945 - Flags: review?(bent.mozilla)
Comment on attachment 8374945 [details] [diff] [review]
fix

Maybe glandium feels more comfortable reviewing this?
Attachment #8374945 - Flags: review?(bent.mozilla) → review?(mh+mozilla)
Can you tell what this is fixing? At quick glance, I don't see what those #defines would be breaking. The functions are inline, and the define will replace the names in both the definition and the callers.
Flags: needinfo?(jacek)
Note that ';' is part of the define. If it wasn't then yes, that would work.

For the reference:
http://repo.or.cz/w/mingw-w64/jacek.git/blob/32259b968875d907252f4f754d696aae114d8434:/mingw-w64-headers/include/strsafe.h#l1944
Flags: needinfo?(jacek)
Comment on attachment 8374945 [details] [diff] [review]
fix

Review of attachment 8374945 [details] [diff] [review]:
-----------------------------------------------------------------

Seems like we just ought to define STRSAFE_NO_DEPRECATE.
Attachment #8374945 - Flags: review?(mh+mozilla) → review-
Attached patch define STRSAFE_NO_DEPRECATE (obsolete) — Splinter Review
Attachment #8374945 - Attachment is obsolete: true
Attachment #8378296 - Flags: review?(mh+mozilla)
Comment on attachment 8378296 [details] [diff] [review]
define STRSAFE_NO_DEPRECATE

Review of attachment 8378296 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/TaskbarPreviewButton.cpp
@@ +4,5 @@
>   * This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#define STRSAFE_NO_DEPRECATE

I think you should just AC_DEFINE it in configure for mingw builds.
Attachment #8378296 - Flags: review?(mh+mozilla) → review-
Attached patch fixSplinter Review
Attachment #8378296 - Attachment is obsolete: true
Attachment #8378881 - Flags: review?(mh+mozilla)
Comment on attachment 8378881 [details] [diff] [review]
fix

Review of attachment 8378881 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +643,5 @@
>                  ;;
>              esac
>          fi
>  
> +        # strsafe.h on mingw uses macros for function deprecation that polutes namespace

pollutes
Attachment #8378881 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/5cfcff375329
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.