Skip to content

bpo-30726: Fix elementtree warnings on Windows due to expat upgrade #2319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jun 23, 2017
Merged

bpo-30726: Fix elementtree warnings on Windows due to expat upgrade #2319

merged 2 commits into from
Jun 23, 2017

Conversation

segevfiner
Copy link
Contributor

We are getting:

Warning	C4005	'HAVE_MEMMOVE': macro redefinition	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\winconfig.h	34	
Warning	C4267	'=': conversion from 'size_t' to 'unsigned char', possible loss of data	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\siphash.h	316	
Warning	C4996	'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.	_elementtree	C:\Users\Segev\prj\python\cpython\Modules\expat\xmlparse.c	796	
Warning	C4005	'HAVE_MEMMOVE': macro redefinition	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\winconfig.h	34	
Warning	C4005	'HAVE_MEMMOVE': macro redefinition	_elementtree	c:\users\segev\prj\python\cpython\modules\expat\winconfig.h	34

And in 64-bit:

c:\users\segev\prj\python\cpython\modules\expat\siphash.h(201): warning C4244: 'initializing': conversion from '__int64' to 'char', possible loss of data

I'm not sure how many Python versions this affects.

@segevfiner segevfiner mentioned this pull request Jun 23, 2017
Merged
@vstinner
Copy link
Member

Can you please rewrite your PR on new master branch, just to add _CRT_SECURE_NO_WARNINGS define? Please explain in the commit message that it's to fix the warning on getenv(). I agree that the usage of getenv() is safe: thanks to the GIL, we only run one thread at the same time, and the getenv() result is not stored but used in the next instruction. So I don't see any risk of race condition here.

@segevfiner segevfiner force-pushed the bpo-30726-elementtree-warnings branch from 45766f8 to 2eb9d7b Compare June 23, 2017 09:02
@segevfiner
bpo-30726: Fix elementtree warnings on Windows
3b89e2e 
Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.
@segevfiner segevfiner force-pushed the bpo-30726-elementtree-warnings branch from 2eb9d7b to 3b89e2e Compare June 23, 2017 09:05
@segevfiner
Copy link
Contributor Author

@Haypo Done. I forgot that I silenced more warnings than those fixed by #2348 when I commented on it. 😛

vstinner
vstinner requested changes Jun 23, 2017
View reviewed changes
PCbuild/_elementtree.vcxproj Outdated
@@ -62,7 +62,8 @@
<ItemDefinitionGroup>
<ClCompile>
<AdditionalIncludeDirectories>..\Modules\expat;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<PreprocessorDefinitions>USE_PYEXPAT_CAPI;XML_STATIC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions>_CRT_SECURE_NO_WARNINGS;USE_PYEXPAT_CAPI;XML_STATIC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<DisableSpecificWarnings>4244;4267;%(DisableSpecificWarnings)</DisableSpecificWarnings>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these warnings? Why do you want to turn them off? If they are related to integer downcast, please keep the warnings. I prefer to fix them rather than make them quiet. See my PR to libexpat. Once it's merged upstream (libexpat), I plan to cherry-pick the fix downstream (CPython).

At least, list these warnings in your commit message with their description.

@segevfiner
Copy link
Contributor Author

segevfiner commented Jun 23, 2017 •
edited
Loading

@Haypo They are:

Warning	C4267	'=': conversion from 'size_t' to 'unsigned char', possible loss of data	_elementtree	siphash.h	316	
siphash.h(201): warning C4244: 'initializing': conversion from '__int64' to 'char', possible loss of data

I think your libexpat/libexpat#58 does fix those. Which is obviously a better way to fix those warnings. I will remove ignoring those warnings.

@vstinner vstinner assigned vstinner and unassigned vstinner Jun 23, 2017
@vstinner vstinner merged commit 87c6555 into python:master Jun 23, 2017
@segevfiner segevfiner deleted the bpo-30726-elementtree-warnings branch June 23, 2017 10:54
vstinner added a commit that referenced this pull request Jun 23, 2017
@vstinner
[3.5] bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)…
f42ce17 
… (#2350)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
vstinner added a commit that referenced this pull request Jun 23, 2017
@vstinner
[3.6] bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)…
d32a059 
… (#2349)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
ned-deily pushed a commit to ned-deily/cpython that referenced this pull request Jul 7, 2017
@vstinner @ned-deily
[3.6] bpo-30726: PCbuild _elementtree: remove duplicate defines (pyth…
5777e79 
…on#2348) (python#2349)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (python#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (python#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
(cherry picked from commit d32a059)
larryhastings pushed a commit that referenced this pull request Jul 12, 2017
@vstinner @larryhastings
[3.4] bpo-29591, bpo-30694: Upgrade Modules/expat to libexpat 2.2.1 (#…
71572bb 
…2164) (#2203)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2 (#2164)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)

* bpo-30694: Upgrade Modules/expat/ to libexpat 2.2.1 (#2300)

New file: Modules/expat/siphash.h.
(cherry picked from commit 5ff7132)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
ned-deily pushed a commit that referenced this pull request Jul 16, 2017
@vstinner @ned-deily
[3.3] bpo-29591, bpo-30694: Upgrade Modules/expat to libexpat 2.2.1 (#…
ab90986 
…2164) (#2204)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2 (#2164)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)

* bpo-30694: Upgrade Modules/expat/ to libexpat 2.2.1 (#2300)

New file: Modules/expat/siphash.h.
(cherry picked from commit 5ff7132)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@segevfiner @vstinner @the-knights-who-say-ni