Skip to content

Add pragma pack directive #557

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.

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

Merged
merged 1 commit into from
Dec 5, 2016
Merged

Conversation

sergiy80
Copy link
Contributor

@sergiy80 sergiy80 commented Dec 3, 2016

Added pragma pack directive to jsoncpp headers.
Related to #458

@sergiy80
Copy link
Contributor Author

sergiy80 commented Dec 3, 2016

To reproduce the issue.
In Visual Studio project (makefiles/msvc2010) just change test_lib_json's Struct Memeber Alignment option from Default (8) to 2, leaving lib_json's option as is (Default).
Without this commit test_lib_json.exe will produce either failed tests or abnormal termination.

@cdunn2001 cdunn2001 merged commit 762ad0f into open-source-parsers:master Dec 5, 2016
@cdunn2001
Copy link
Contributor

Was this a problem anywhere other than Windows/VisualStudio?

@sergiy80
Copy link
Contributor Author

sergiy80 commented Dec 5, 2016

Also I could reproduce seg fault error compiling jsoncpp using gcc and clang under MSYS2 environment.
Beause linux standard system headers do not have packing guards (unlike VS standard headers), it is not possible just to use packing option other then default (8) for whole project.
To reproduce the crash need to modify jsontest.cpp and main.cpp in test_lib_json project to exclude standard headers from changing packing opton.
The diffs are below.
With these changes in the test project the results are the same as for VS project, failed tests and segmentation fault error.

diff --git a/src/test_lib_json/jsontest.cpp b/src/test_lib_json/jsontest.cpp
index 4c10a37..08e193f 100644
--- a/src/test_lib_json/jsontest.cpp
+++ b/src/test_lib_json/jsontest.cpp
@@ -68,6 +68,8 @@
 #include <windows.h>
 #endif // if defined(_WIN32)
 
+#pragma pack(2)
+
 namespace JsonTest {
 
 // class TestResult
diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp
index 47cd981..623a692 100644
--- a/src/test_lib_json/main.cpp
+++ b/src/test_lib_json/main.cpp
@@ -3,15 +3,19 @@
 // recognized in your jurisdiction.
 // See file LICENSE for detail or copy at http://jsoncpp.sourceforge.net/LICENSE
 
-#include "jsontest.h"
-#include <json/config.h>
-#include <json/json.h>
+
 #include <cstring>
 #include <limits>
 #include <sstream>
 #include <string>
 #include <iomanip>
 
+#pragma pack(2)
+#include "jsontest.h"
+#include <json/config.h>
+#include <json/json.h>
+
 // Make numeric limits more convenient to talk about.
 // Assumes int type in 32 bits.
 #define kint32max Json::Value::maxInt

@cdunn2001
Copy link
Contributor

Thanks. Very interesting. I did not know about this pragma.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants