Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(151)

Issue 1072653003: Fix errors caused by illegal #define of snprintf. (Closed)

Created:
5 years, 8 months ago by brucedawson
Modified:
5 years, 8 months ago
Reviewers:
cpu_(ooo_6.6-7.5)
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix errors caused by illegal #define of snprintf. Because VS has, for a long time, not had snprintf some developers have fixed this with #define snprintf _snprintf. This is scary because the two functions behave differently, and it also breaks with VS 2015 which does implement snprintf. So, the #defines all need a version check. The errors are: stdio.h(1926): warning C4005: 'snprintf': macro redefinition libxml\win32\config.h(98): note: see previous definition of 'snprintf' libxml\src\include\win32config.h(98): note: see previous def of 'snprintf' stdio.h(1929): fatal error C1189: #error: Macro definition of snprintf conflicts with Standard Library function declaration R=cpu@chromium.org BUG=440500 Committed: https://crrev.com/548cc22b487981ecd15a2e0b869a15309446e6f4 Cr-Commit-Position: refs/heads/master@{#325832}

Patch Set 1 #

Patch Set 2 : Modify README.chromium and add patches. #

Patch Set 3 : Change tense of comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -0 lines) Patch
M third_party/libxml/README.chromium View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A third_party/libxml/patches/snprintf_config View 1 1 chunk +14 lines, -0 lines 0 comments Download
A third_party/libxml/patches/snprintf_win32config View 1 1 chunk +14 lines, -0 lines 0 comments Download
M third_party/libxml/src/include/win32config.h View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/libxml/win32/config.h View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
brucedawson
5 years, 8 months ago (2015-04-08 22:37:27 UTC) #1
cpu_(ooo_6.6-7.5)
lgtm but same deal you need to add the patch to \patches
5 years, 8 months ago (2015-04-09 00:33:49 UTC) #2
brucedawson
Patches and README.chromium changes made. PTAL.
5 years, 8 months ago (2015-04-16 20:41:18 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1072653003/40001
5 years, 8 months ago (2015-04-20 10:54:35 UTC) #6
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 8 months ago (2015-04-20 12:09:30 UTC) #7
commit-bot: I haz the power
5 years, 8 months ago (2015-04-20 12:10:20 UTC) #8
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/548cc22b487981ecd15a2e0b869a15309446e6f4
Cr-Commit-Position: refs/heads/master@{#325832}

Powered by Google App Engine
This is Rietveld 408576698