Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1836443002:
Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS (Closed)
DescriptionRe-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS
The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium
code as pdfium code itself is warning free;
For standalone build, the macro may suppress warnings from third
party code. But it has no use now as it is shadowed by disabled 4996
warnings.
This is one of the efforts to re-enable warnings for pdfium code.
BUG=pdfium:29
R=brucedawson@chromium.org
Committed: https://pdfium.googlesource.com/pdfium/+/85ff9870a48e3ad69228e7fa813b558f51483028
Patch Set 1 #
Messages
Total messages: 20 (13 generated)
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed as the warning 4996 is disabled. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed as the warning 4996 is disabled. BUG=pdfium:29 ==========
weili@chromium.org changed reviewers: + tsepez@chromium.org
PTAL, thanks
tsepez@chromium.org changed reviewers: + brucedawson@chromium.org
Deferring to Bruce.
If we remove _CRT_SECURE_NO_WARNINGS then we make it harder to ever enable C4996 warnings in the future. C4996 can be valuable for other types of deprecation even if we don't want it for sprintf and what-not. That said, I'm not saying this change is wrong. It's not an unreasonable change by any means. Has this been tested in the context of a full Chromium build?
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed as the warning 4996 is disabled. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed for chromium code as pdfium code is warning free; for standalone build, the macro also has no use since warning 4996 is disabled. BUG=pdfium:29 ==========
On 2016/03/25 17:56:35, brucedawson wrote: > If we remove _CRT_SECURE_NO_WARNINGS then we make it harder to ever enable C4996 > warnings in the future. C4996 can be valuable for other types of deprecation > even if we don't want it for sprintf and what-not. > > That said, I'm not saying this change is wrong. It's not an unreasonable change > by any means. > > Has this been tested in the context of a full Chromium build? Yes, this has been tested in full Chromium builds. Sorry, I didn't make this more clear. Now pdfium code (at least the part compiles) is already crt unsecured warning free. So it is safe to remove the macro from BUILD.gn and pdfium.gyp. For the one in fx_system.h, it may help suppress some warnings raised from third_part code in a standalone build. However, it is now shadowed by disabled 4996 warnings. So it is not very useful. I can revert this one or move it outside the header if it is more desirable.
Please reformat the description to fit in 72 characters - standard git commit message maximum length. This is my favorite article on how to write great git commit messages: http://chris.beams.io/posts/git-commit/ lgtm
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed for chromium code as pdfium code is warning free; for standalone build, the macro also has no use since warning 4996 is disabled. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed for chromium code as pdfium code is warning free; for standalone build, the macro also has no use since warning 4996 is disabled. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed for chromium code as pdfium code is warning free; for standalone build, the macro also has no use since warning 4996 is disabled. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed for chromium code as pdfium code is warning free; for standalone build, the macro also has no use since warning 4996 is disabled. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS macro The macro is no longer needed for chromium code as pdfium code is warning free; for standalone build, the macro also has no use since warning 4996 is disabled. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third_part code. But it has no use as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third_part code. But it has no use as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third_part code. But it has no use now as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third_part code. But it has no use now as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third_part code. But it has no use now as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third_part code. But it has no use now as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. This is one of the efforts to re-enable warnings for pdfium code. BUG=pdfium:29 ==========
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. This is one of the efforts to re-enable warnings for pdfium code. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. This is one of the efforts to re-enable warnings for pdfium code. BUG=pdfium:29 ==========
On 2016/03/29 21:02:41, brucedawson wrote: > Please reformat the description to fit in 72 characters - standard git commit > message maximum length. > > This is my favorite article on how to write great git commit messages: > http://chris.beams.io/posts/git-commit/ > > lgtm thanks, done.
Description was changed from ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. This is one of the efforts to re-enable warnings for pdfium code. BUG=pdfium:29 ========== to ========== Re-enable warning 4005 and remove _CRT_SECURE_NO_WARNINGS The macro _CRT_SECURE_NO_WARNINGS is no longer needed for chromium code as pdfium code itself is warning free; For standalone build, the macro may suppress warnings from third party code. But it has no use now as it is shadowed by disabled 4996 warnings. This is one of the efforts to re-enable warnings for pdfium code. BUG=pdfium:29 R=brucedawson@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/85ff9870a48e3ad69228e7fa813b558f5148... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as 85ff9870a48e3ad69228e7fa813b558f51483028 (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
