[geode-native] branch develop updated: GEODE-3453: Fix native client function tests to use updated API
This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new a2e1f10 GEODE-3453: Fix native client function tests to use updated
API
a2e1f10 is described below
commit a2e1f102b0ac743b929f826147b34a7bcde3f97e
Author: David Kimura
AuthorDate: Wed Aug 16 17:35:33 2017 -0700
GEODE-3453: Fix native client function tests to use updated API
---
tests/javaobject/GetFunctionExeHA.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/javaobject/GetFunctionExeHA.java
b/tests/javaobject/GetFunctionExeHA.java
index 62216c0..ece5f33 100644
--- a/tests/javaobject/GetFunctionExeHA.java
+++ b/tests/javaobject/GetFunctionExeHA.java
@@ -35,7 +35,7 @@ public class GetFunctionExeHA extends FunctionAdapter
implements Declarable{
RegionFunctionContext context = (RegionFunctionContext)fc;
System.out.println("Data set :: " + context.getDataSet());
Region region = PartitionRegionHelper.getLocalDataForContext(context);
-Set keys = region.keys();
+Set keys = region.keySet();
Iterator itr = keys.iterator();
ResultSender sender = context.getResultSender();
Object k = null;
--
To stop receiving notification emails like this one, please contact
['"dev@geode.apache.org" '].
[geode-native] branch develop updated: GEODE-3455: Update Solaris build images.
This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new 0fba5ce GEODE-3455: Update Solaris build images.
0fba5ce is described below
commit 0fba5cee59d5d53eaab6cc9396eab45df4b8d79c
Author: Jacob Barrett
AuthorDate: Wed Aug 2 12:32:34 2017 -0700
GEODE-3455: Update Solaris build images.
- Fixes Solaris SPARC image.
- Addes developer images.
---
packer/build-solaris-sparc.json | 10 +
packer/build-solaris-x86.json | 7 +++---
packer/dev-solaris-sparc.json | 38 +
packer/dev-solaris-x86.json | 38 +
packer/solaris/install-cmake.sh | 8 ---
packer/solaris/install-solarisstudio.sh | 1 -
6 files changed, 91 insertions(+), 11 deletions(-)
diff --git a/packer/build-solaris-sparc.json b/packer/build-solaris-sparc.json
index 34aea96..0ce5eec 100644
--- a/packer/build-solaris-sparc.json
+++ b/packer/build-solaris-sparc.json
@@ -1,8 +1,9 @@
{
"variables":{
"image_name":"build-solaris-sparc",
-"openstack_source_image":"7e1e66f1-8256-4e29-a314-ee7f54b2f720",
-"vmware_source_image_name":"X.vmx",
+"openstack_source_image":"",
+"openstack_flavor":"Oracle Solaris non-global zone - tiny",
+"vmware_source_image_name":"",
"gemfire_archive":"gemfire.tar.gz",
"pkg_oracle_com_certificate":"pkg.oracle.com.certificate.pem",
"pkg_oracle_com_key":"pkg.oracle.com.key.pem"
@@ -18,7 +19,7 @@
"ssh_username":"root",
"image_name":"native-{{user `version`}}-{{user `image_name`}}
{{timestamp}}",
"source_image":"{{user `openstack_source_image`}}",
- "flavor":"Oracle Solaris non-global zone - tiny",
+ "flavor":"{{user `openstack_flavor`}}",
"insecure":"true"
}
],
@@ -38,7 +39,8 @@
"scripts":[
"solaris/install-opencsw.sh",
"solaris/install-build-tools.sh",
-"solaris/install-solarisstudio.sh"
+"solaris/install-solarisstudio.sh",
+"solaris/install-cmake.sh"
]
},
{
diff --git a/packer/build-solaris-x86.json b/packer/build-solaris-x86.json
index b01519e..6a55c82 100644
--- a/packer/build-solaris-x86.json
+++ b/packer/build-solaris-x86.json
@@ -1,8 +1,9 @@
{
"variables":{
"image_name":"build-solaris-x86",
-"openstack_source_image":"c0df7ff9-fc8f-4220-ac1f-fd24924dfe7a",
-"vmware_source_image_name":"X.vmx",
+"openstack_source_image":"",
+"openstack_flavor":"Oracle Solaris non-global zone - tiny",
+"vmware_source_image_name":"",
"gemfire_archive":"gemfire.tar.gz",
"pkg_oracle_com_certificate":"pkg.oracle.com.certificate.pem",
"pkg_oracle_com_key":"pkg.oracle.com.key.pem"
@@ -18,7 +19,7 @@
"ssh_username":"root",
"image_name":"native-{{user `version`}}-{{user `image_name`}}
{{timestamp}}",
"source_image":"{{user `openstack_source_image`}}",
- "flavor":"Oracle Solaris non-global zone - tiny",
+ "flavor":"{{user `openstack_flavor`}}",
"insecure":"true"
}
],
diff --git a/packer/dev-solaris-sparc.json b/packer/dev-solaris-sparc.json
new file mode 100644
index 000..180e8e3
--- /dev/null
+++ b/packer/dev-solaris-sparc.json
@@ -0,0 +1,38 @@
+{
+ "variables":{
+"image_name":"dev-solaris-sparc",
+"openstack_source_image":"",
+"openstack_flavor":"Oracle Solaris non-global zone - tiny",
+"vmware_source_image_name":""
+ },
+ "builders":[
+{
+ "type":"openstack",
+ "identity_endpoint":"{{user `openstack_identity_endpoint`}}",
+ "tenant_name":"{{user `openstack_tenant_name`}}",
+ "username":"{{user `openstack_username`}}",
+ "password":"{{user `openstack_password`}}",
+ "region":"{{user `openstack_region`}}",
+ "ssh_username":"root",
+ "image_name":"native-{{user `version`}}-{
[geode-native] branch develop updated: GEODE-3457: Linking fails on Solaris SPARC due to -xatomic=none option
This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new 8e4fd71 GEODE-3457: Linking fails on Solaris SPARC due to
-xatomic=none option
8e4fd71 is described below
commit 8e4fd714f7d317484cacd92f54539eb6655681ad
Author: David Kimura
AuthorDate: Thu Aug 17 14:11:19 2017 -0700
GEODE-3457: Linking fails on Solaris SPARC due to -xatomic=none option
---
CMakeLists.txt | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index bb40517..abb7a65 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -166,7 +166,12 @@ add_library(c++11 INTERFACE)
if(CMAKE_CXX_COMPILER_ID STREQUAL "SunPro")
# Force linker to error on undefined symbols in shared libraries
- set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -z defs
-xatomic=none")
+ set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -z defs")
+ if (PRODUCT_SYSTEM_NAME STREQUAL "solaris-sparc")
+set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}
-xatomic=studio")
+ elseif(PRODUCT_SYSTEM_NAME STREQUAL "solaris-x86")
+set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -xatomic=none")
+ endif()
# TODO cmake find a better way to set runtime libraries
# C++11 requires these libraries, treat -std=c++11 as library
#TODO look into CMAKE_CXX_STANDARD_LIBRARIES
--
To stop receiving notification emails like this one, please contact
['"dev@geode.apache.org" '].
[geode-native] branch develop updated: GEODE-3497: Fix failing test on Solaris SPARC
This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new cc6dafe GEODE-3497: Fix failing test on Solaris SPARC
cc6dafe is described below
commit cc6dafee2cd82fa770686fa9b9d87ba4eeb3
Author: David Kimura
AuthorDate: Mon Aug 21 13:05:49 2017 -0700
GEODE-3497: Fix failing test on Solaris SPARC
---
cppcache/test/ClientProxyMembershipIDFactoryTest.cpp | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/cppcache/test/ClientProxyMembershipIDFactoryTest.cpp
b/cppcache/test/ClientProxyMembershipIDFactoryTest.cpp
index b9fec88..d593812 100644
--- a/cppcache/test/ClientProxyMembershipIDFactoryTest.cpp
+++ b/cppcache/test/ClientProxyMembershipIDFactoryTest.cpp
@@ -26,17 +26,18 @@ using namespace apache::geode::client;
TEST(ClientProxyMembershipIDFactoryTest, testCreate) {
ClientProxyMembershipIDFactory factory("myDs");
- auto id = factory.create("myHost", 1, 2, "myClientID", 3);
+ auto hostAddr = htonl(1);
+ auto id = factory.create("myHost", hostAddr, 2, "myClientID", 3);
ASSERT_NE(nullptr, id);
EXPECT_EQ("myDs", id->getDSName());
- EXPECT_EQ(1, static_cast(*id->getHostAddr()));
+ EXPECT_EQ(hostAddr, *reinterpret_cast(id->getHostAddr()));
EXPECT_EQ(4, id->getHostAddrLen());
EXPECT_EQ(2, id->getHostPort());
auto uniqueTag = id->getUniqueTag();
ASSERT_NE("", uniqueTag);
- EXPECT_EQ(std::string(":1:0:0:0:2:myDs:").append(uniqueTag),
+ EXPECT_EQ(std::string(":0:0:0:1:2:myDs:").append(uniqueTag),
id->getHashKey());
EXPECT_TRUE(std::regex_search(
id->getDSMemberIdForThinClientUse(),
--
To stop receiving notification emails like this one, please contact
['"dev@geode.apache.org" '].
[GitHub] geode-native pull request #95: GEODE-2832: Fixing the link of README.md
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/95#discussion_r115007123 --- Diff: README.md --- @@ -24,3 +24,33 @@ Native Client applications can be written in these client technologies: * [C++](https://isocpp.org) * [C#](https://msdn.microsoft.com/en-us/library/ms228593.aspx) + +## Export Control + +This distribution includes cryptographic software. +The country in which you currently reside may have restrictions +on the import, possession, use, and/or re-export to another country, +of encryption software. BEFORE using any encryption software, +please check your country's laws, regulations and policies +concerning the import, possession, or use, and re-export of +encryption software, to see if this is permitted. +See <http://www.wassenaar.org/> for more information. + +The U.S. Government Department of Commerce, Bureau of Industry and Security (BIS), +has classified this software as Export Commodity Control Number (ECCN) 5D002.C.1, +which includes information security software using or performing +cryptographic functions with asymmetric algorithms. +The form and manner of this Apache Software Foundation distribution makes +it eligible for export under the License Exception +ENC Technology Software Unrestricted (TSU) exception +(see the BIS Export Administration Regulations, Section 740.13) +for both object code and source code. + +The following provides more details on the included cryptographic software: + +* Apache Geode is designed to be used with + [Java Secure Socket Extension](https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html) (JSSE) and --- End diff -- This README is for geode-native. It does not run in Java and therefor should not have the statements about JCE. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #95: GEODE-2832: Fixing the link of README.md
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/95#discussion_r115026056 --- Diff: README.md --- @@ -24,3 +24,33 @@ Native Client applications can be written in these client technologies: * [C++](https://isocpp.org) * [C#](https://msdn.microsoft.com/en-us/library/ms228593.aspx) + +## Export Control + +This distribution includes cryptographic software. +The country in which you currently reside may have restrictions +on the import, possession, use, and/or re-export to another country, +of encryption software. BEFORE using any encryption software, +please check your country's laws, regulations and policies +concerning the import, possession, or use, and re-export of +encryption software, to see if this is permitted. +See <http://www.wassenaar.org/> for more information. + +The U.S. Government Department of Commerce, Bureau of Industry and Security (BIS), +has classified this software as Export Commodity Control Number (ECCN) 5D002.C.1, +which includes information security software using or performing +cryptographic functions with asymmetric algorithms. +The form and manner of this Apache Software Foundation distribution makes +it eligible for export under the License Exception +ENC Technology Software Unrestricted (TSU) exception +(see the BIS Export Administration Regulations, Section 740.13) +for both object code and source code. + +The following provides more details on the included cryptographic software: + +* Apache Geode is designed to be used with + [Java Secure Socket Extension](https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html) (JSSE) and --- End diff -- Oh, I see, you merged rather rather than rebased. I think rather than going through all the paperwork I would just correct it inline with this issue. Either way is fine. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #95: GEODE-2832: Fixing the link of README.md
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/95 @masaki-yamakawa For future reference, please rebase your branch before pull requests and include the `GEODE-XX: ` prefix in your first line commit statement. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #99: GEODE-2741: Remove custom shared painter and ...
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/99 GEODE-2741: Remove custom shared painter and replace with std::shared_ptr. Tests pass on Linux, Windows, Solaris and Mac. Please review. You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native wip/shared_ptr Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/99.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #99 commit c0098121eb9b8ee7d537ac0d0bb767ebb6d46927 Author: Jacob Barrett Date: 2017-03-29T00:34:29Z GEODE-2741: Remove custom shared pointer from cppcache commit 6027574a2078b35fadcef450020af775415f1c36 Author: Jacob Barrett Date: 2017-04-21T22:59:27Z GEODE-2741: Remove custom shared pointer from clicache commit 9a06e1681438c01e3f721f4a3f7b7576bedefd2a Author: Jacob Barrett Date: 2017-05-16T13:24:16Z GEODE-2741: Backs out workaround for .NET Environment.Exit issues. commit c6fdafe5793afd057e95b5fbb3d07b88665d1043 Author: Jacob Barrett Date: 2017-05-16T20:29:56Z GEODE-2741: Fix casting issues between generics. commit 3cb20df08f6b5d624b75609e54354a077b5fcc03 Author: Jacob Barrett Date: 2017-05-16T22:29:14Z GEODE-2741: Fixes potential memory leak on exception. commit d58a5e30d42ae38fd9a439fe875bb9096cdde386 Author: Jacob Barrett Date: 2017-05-17T03:26:35Z GEODE-2741: Fixes inclusion of unmanaged headers. commit 767033e5cfd70b3ad41cac67d63db464b3697a85 Author: Jacob Barrett Date: 2017-05-17T04:09:22Z GEODE-2741: Workaround for static de-init issues with CLR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #102: GEODE-2741: Adding a warning for 64bit Windo...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/102#discussion_r121248442
--- Diff: src/CMakeLists.txt ---
@@ -17,6 +17,15 @@ project(nativeclient)
list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/../cmake)
+if(CMAKE_GENERATOR MATCHES Win64*)
+ if ((CMAKE_GENERATOR MATCHES "Visual Studio") AND
(CMAKE_GENERATOR_TOOLSET STREQUAL ""))
--- End diff --
Why not just set the `CMAKE_GENERATOR_TOOLSET`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #103: GEODE-2525: Removes MersenneTwister sources.
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/103 GEODE-2525: Removes MersenneTwister sources. Can I get a quick review before I merge. You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native feature/GEODE-2525-cp Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/103.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #103 commit 46a141ca58bdaba3e162dc3e0848a725183169d4 Author: Jacob Barrett Date: 2017-03-07T16:35:33Z GEODE-2525: Removes MersenneTwister sources. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #103: GEODE-2525: Removes MersenneTwister sources.
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/103#discussion_r123866557
--- Diff: src/tests/cpp/fwklib/GsRandom.cpp ---
@@ -18,60 +18,16 @@
#include "GsRandom.hpp"
#include
-#include
-#include
namespace apache {
namespace geode {
namespace client {
namespace testframework {
-using util::concurrent::spinlock_mutex;
-
-GsRandom *GsRandom::singleton = 0;
-MTRand GsRandom::gen;
-int32_t GsRandom::seedUsed = -101;
-spinlock_mutex GsRandom::lck;
-
-/**
- * Creates a new random number generator using a single
- * int32_t seed.
- *
- * @param seed the initial seed.
- * @see java.util.Random#Random(int32_t)
- */
-GsRandom *GsRandom::getInstance(int32_t seed) {
- if (singleton == 0) {
-setInstance(seed);
- } else {
-std::lock_guard guard(lck);
-setSeed(seed);
- }
- return singleton;
-}
-
-void GsRandom::setInstance(int32_t seed) {
- std::lock_guard guard(lck);
- if (singleton == 0) {
-singleton = new GsRandom();
-if (seed != -1) {
- singleton->gen.seed(seed);
-} else {
- singleton->gen.seed();
-}
-seedUsed = seed;
- }
-}
-
-void GsRandom::setSeed(int32_t seed) {
- if (seed != seedUsed) {
-if (seed != -1) {
- singleton->gen.seed(seed);
-} else {
- singleton->gen.seed();
-}
-seedUsed = seed;
- }
+GsRandom &GsRandom::getInstance() {
+ // C++11 initializes statics threads safe
--- End diff --
Yeah probably not. I can kill it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #104: Develop
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/104 It looks like your reverts were to undo changes to .gitignore. I'd suggest not building from within the source tree at all. If you do there is an ignore already for "build" that you can use as your build directory. CMake promotes building outside the source directory, you can generate your project files from any location. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #104: Develop
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/104 Please open a JIRA and include it in your commit message. GEODE-1234: blah blah --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #105: GEODE-2891 connect-timeout violation in C++ ...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/105#discussion_r125184959
--- Diff: src/cppcache/src/TcrMessage.hpp ---
@@ -171,7 +173,8 @@ class CPPCACHE_EXPORT TcrMessage {
GET_DURABLE_CQS_DATA_ERROR = 106,
GET_ALL_WITH_CALLBACK = 107,
PUT_ALL_WITH_CALLBACK = 108,
-REMOVE_ALL = 109
+ REMOVE_ALL = 109,
+ HANDSHAKE = 110
--- End diff --
These numbers correspond to protocol message numbers on the server. We can
just add one here and expect it not cause issues later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #105: GEODE-2891 connect-timeout violation in C++ ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/105#discussion_r125184965 --- Diff: src/cppcache/src/TcrMessage.hpp --- @@ -44,6 +44,8 @@ #include #include +// --- End diff -- Clean this up. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #105: GEODE-2891 connect-timeout violation in C++ ...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/105#discussion_r125184798
--- Diff: src/cppcache/src/TcrConnection.cpp ---
@@ -318,7 +318,9 @@ bool TcrConnection::InitTcrConnection(
LOGFINE("Attempting handshake with endpoint %s for %s%s connection",
endpoint,
isClientNotification ? (isSecondary ? "secondary " : "primary ")
: "",
isClientNotification ? "subscription" : "client");
- ConnErrType error = sendData(data, msgLengh, connectTimeout, false);
+ // GT GEODE-2891
+ //ConnErrType error = sendData( data, msgLengh, connectTimeout, false );
--- End diff --
Do not leave commented out sources, this is what revision control is for.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #105: GEODE-2891 connect-timeout violation in C++ ...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/105#discussion_r125184917
--- Diff: src/cppcache/src/TcrConnection.cpp ---
@@ -703,15 +717,21 @@ inline ConnErrType TcrConnection::sendData(uint32_t&
timeSpent,
is not public yet*/
notPublicApiWithTimeout == TcrMessage::EXECUTE_FUNCTION ||
notPublicApiWithTimeout == TcrMessage::EXECUTE_REGION_FUNCTION ||
-notPublicApiWithTimeout ==
-TcrMessage::EXECUTE_REGION_FUNCTION_SINGLE_HOP) {
- // then app has set timeout in millis, change it to microSeconds
- sendTimeoutSec = sendTimeoutSec * 1000;
- isPublicApiTimeout = true;
- LOGDEBUG("sendData2 %d ", sendTimeoutSec);
-} else {
- sendTimeoutSec = sendTimeoutSec * 1000;
-}
+ notPublicApiWithTimeout ==
TcrMessage::EXECUTE_REGION_FUNCTION_SINGLE_HOP ||
+ // GT GEODE-2891
+ notPublicApiWithTimeout == TcrMessage::HANDSHAKE)
+ {
+ // then app has set timeout in millis, change it to microSeconds
+ sendTimeoutSec = sendTimeoutSec * 1000;
+ isPublicApiTimeout = true;
+ LOGDEBUG("sendData2 %d ", sendTimeoutSec);
+ }
--- End diff --
Formatting does not conform to [Google C++ Style
Guide](https://google.github.io/styleguide/cppguide.html).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #105: GEODE-2891 connect-timeout violation in C++ ...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/105#discussion_r125184790
--- Diff: src/cppcache/src/TcrConnection.cpp ---
@@ -318,7 +318,9 @@ bool TcrConnection::InitTcrConnection(
LOGFINE("Attempting handshake with endpoint %s for %s%s connection",
endpoint,
isClientNotification ? (isSecondary ? "secondary " : "primary ")
: "",
isClientNotification ? "subscription" : "client");
- ConnErrType error = sendData(data, msgLengh, connectTimeout, false);
+ // GT GEODE-2891
--- End diff --
Do not include comments with your name or the ticket number.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #105: GEODE-2891 connect-timeout violation in C++ Native ...
Github user pivotal-jbarrett commented on the issue:
https://github.com/apache/geode-native/pull/105
> I suggest to fix the issue by defining handshake pseudo message within
the range probably defined for such pseudo messages by original design i.e.:
> typedef enum {
>/* Server couldn't read message; handle it like a server side
> exception that needs retries */
>HANDSHAKE = -3
>NOT_PUBLIC_API_WITH_TIMEOUT = -2,
I think that would certainly work better but I think ultimately the fix is
to remove the ambiguity between the C++ API and the Server API by strongly
typing the duration values as addressed in the mentioned tickets. All duration
values can be cast to the internal API supported unit without any ambiguity.
After which there is no need to create this pseudo message to change the unit
of the ambiguous int value.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #105: GEODE-2891 connect-timeout violation in C++ Native ...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/105 @gregt5259 please close this pull request since you opened #106. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/106 @echobravopapa the branching scheme only applies to committers working directly with the Geode repositories. Non-committers use pull requests which render the branching/girflow requirement null. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #106: GEODE-2891 connect-timeout violation in C++ Native ...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/106 @gregt5259 This is a solution to the problem but not the solution we as committers are comfortable committing as it directly conflicts with the correct change, which is to use type safe durations rather than magic number math and system wide properties to create a confusing array of time values. If you want this change sooner than later you could implement it using std::chrono::duration as outlined in GEODE-3137 or maintain a fork with your change in it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #108: GEODE-3135 Update Geode Client docs OpenSSL ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/108#discussion_r126114295 --- Diff: docs/geode-native-docs/introduction/quickstart.html.md.erb --- @@ -53,7 +53,7 @@ for the latest Java version for your operating system. See the installation info Geode User's Guide for the versions of Java that are compatible with Geode. - **Security Toolkit** (optional): The QuickStart security examples rely on a security plugin, `securityImpl`, which is based on the OpenSSL toolkit. -If you plan to run the security examples, download the latest version of release 1.0.1 (must be 1.0.1) from the [OpenSSL website] (http://www.openssl.org/source/). +If you plan to run the security examples, download the latest version of release 1.0.2 (must be 1.0.2) from the [OpenSSL website] (http://www.openssl.org/source/). --- End diff -- For maintenance purposes it seems cleaner to have a single section in our docs that talk about external requirements like OpenSSL and then have all the places talk about that requirement to reference back to that page. This way when we change versions, instructions, etc., we don't have to change multiple places in the docs. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #108: GEODE-3135 Update Geode Client docs OpenSSL ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/108#discussion_r126114552 --- Diff: docs/geode-native-docs/security/sslclientserver.html.md.erb --- @@ -27,7 +27,7 @@ The open-source OpenSSL toolkit provides a full-strength general purpose cryptog Follow these instructions to download and install OpenSSL for your specific operating system. -The native client requires OpenSSL 1.0.1t or later. For Windows platforms, you can use either the regular or the OpenSSL 1.0.1t "Light" version. +The native client requires OpenSSL 1.0.2l or later. For Windows platforms, you can use either the regular or the "Light" version. --- End diff -- Same line as in first file. Lets consolidate all this into a single doc around SSL and have these pages reference that doc. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #108: GEODE-3135 Update Geode Client docs OpenSSL ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/108#discussion_r126113664 --- Diff: docs/geode-native-docs/introduction/client-configurations.html.md.erb --- @@ -157,7 +157,7 @@ The following libraries are external dependencies of the client library, `libgfc If you plan on using SSL in your Geode client and server deployment, you will need to download and install OpenSSL. -The client requires OpenSSL 1.0.1t or later. For Windows platforms, you can use either the regular or the OpenSSL "Light" version. +The client requires OpenSSL 1.0.2l or later. For Windows platforms, you can use either the regular or the OpenSSL "Light" version. --- End diff -- Somehow we need to convey that it is restricted to the 1.0.2 releases of OpenSSL. It would not work for someone to install 1.1.0 despite it meeting the terms of "later". --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #108: GEODE-3135 Update Geode Client docs OpenSSL ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/108#discussion_r126114762 --- Diff: docs/geode-native-docs/security/sslclientserver.html.md.erb --- @@ -42,17 +42,17 @@ To install OpenSSL: 2. Extract the archive in a directory of your choice. For example: ``` -$ tar xvzf openssl-1.0.1u.tar.gz -x openssl-1.0.1u/ACKNOWLEDGMENTS -x openssl-1.0.1u/apps/ -x openssl-1.0.1u/apps/app_rand.c +$ tar xvzf openssl-1.0.2l.tar.gz --- End diff -- Leave off the version numbers for all of these examples. These bits of example on OpenSSL setup should be in a separate doc. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r126115701
--- Diff: src/cppcache/include/geode/Struct.hpp ---
@@ -138,7 +138,7 @@ class CPPCACHE_EXPORT Struct : public Serializable {
* Returns the name of the field corresponding to the index number in the
* Struct
*/
- virtual const char* getFieldName(const int32_t index) const;
+ virtual const std::string* getFieldName(const int32_t index) const;
--- End diff --
I am too not a huge fan of returning `std::string*` but the current
behavior is to return `nullptr` if the field does not exist at that index.
@dgkimura would you be happier if this method threw
[`out_of_range`](http://en.cppreference.com/w/cpp/error/out_of_range)? I think
I might like that better than null.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r126116586
--- Diff: src/cppcache/integration-test/testThinClientRemoteQuerySS.cpp ---
@@ -56,59 +57,63 @@ const wchar_t* checkNullString(const wchar_t* str) {
return ((str == nullptr) ? L"(null)" : str);
}
+std::string checkNullString(const std::string* str) {
+return ((str == nullptr) ? "(null)" : *str);
+}
+
void _printFields(CacheablePtr field, Struct* ssptr, int32_t& fields) {
if (auto portfolio = std::dynamic_pointer_cast(field)) {
printf(" pulled %s :- ID %d, pkid %s\n",
- checkNullString(ssptr->getFieldName(fields)),
portfolio->getID(),
+ checkNullString(ssptr->getFieldName(fields)).c_str(),
portfolio->getID(),
--- End diff --
I think I would just rather see the tests not log so much crap and if they
do that they use a logger with stream functionality.
```
if (log.debug) {
log.debug << "Doing some logging. somevar=" << someVar << ",
someOtherVar=" << someOtherVar;
}
```
ð
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #102: GEODE-2741: Adding a warning for 64bit Windows Tool...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/102 @mhansonp please follow up or close this pull request. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #110: GEODE-3135 Update Geode Client docs OpenSSL ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/110#discussion_r126254216 --- Diff: docs/geode-native-docs/introduction/client-configurations.html.md.erb --- @@ -157,7 +157,7 @@ The following libraries are external dependencies of the client library, `libgfc If you plan on using SSL in your Geode client and server deployment, you will need to download and install OpenSSL. -The client requires OpenSSL 1.0.1t or later. For Windows platforms, you can use either the regular or the OpenSSL "Light" version. +The client requires OpenSSL version 1.0.2. For Windows platforms, you can use either the regular or the OpenSSL "Light" version. --- End diff -- I still feel like it would be better to save that "For Windows..." statement for a common area about where to get OpenSSL. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #110: GEODE-3135 Update Geode Client docs OpenSSL ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/110#discussion_r126254300 --- Diff: docs/geode-native-docs/introduction/client-configurations.html.md.erb --- @@ -157,7 +157,7 @@ The following libraries are external dependencies of the client library, `libgfc If you plan on using SSL in your Geode client and server deployment, you will need to download and install OpenSSL. -The client requires OpenSSL 1.0.1t or later. For Windows platforms, you can use either the regular or the OpenSSL "Light" version. +The client requires OpenSSL version 1.0.2. For Windows platforms, you can use either the regular or the OpenSSL "Light" version. --- End diff -- Maybe just link to the sslclientserver.html.md.erb file below. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #102: GEODE-2741: Adding a warning for 64bit Windows Tool...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/102 @mhansonp you have conflicts, can you rebase and force push. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r126792435
--- Diff: src/cppcache/src/Struct.cpp ---
@@ -96,16 +99,16 @@ Serializable* Struct::fromData(DataInput& input) {
return this;
}
-const char* Struct::getFieldName(const int32_t index) const {
+const std::string& Struct::getFieldName(const int32_t index) const {
if (m_parent != nullptr) {
return m_parent->getFieldName(index);
} else {
for (const auto& iter : m_fieldNames) {
- if ((iter.second)->value() == index) return iter.first->asChar();
+ if (iter.second == index) return (iter.first);
}
}
- return nullptr;
+ return std::string("");
--- End diff --
What about throwing
[`std::out_of_range`](http://en.cppreference.com/w/cpp/error/out_of_range). I
don't like the idea of checking for empty string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r126792525
--- Diff: src/cppcache/src/StructSetImpl.cpp ---
@@ -78,13 +78,11 @@ int32_t StructSetImpl::getFieldIndex(const char*
fieldname) {
}
}
-const char* StructSetImpl::getFieldName(int32_t index) {
- for (std::map::iterator iter =
- m_fieldNameIndexMap.begin();
- iter != m_fieldNameIndexMap.end(); ++iter) {
-if (iter->second == index) return iter->first.c_str();
+const std::string& StructSetImpl::getFieldName(int32_t index) {
+ for (const auto& iter : m_fieldNameIndexMap) {
+if (iter.second == index) return (iter.first);
}
- return nullptr;
+ return std::string("");
--- End diff --
`std::out_of_range`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r127278631
--- Diff: src/cppcache/src/StructSetImpl.cpp ---
@@ -78,13 +78,12 @@ int32_t StructSetImpl::getFieldIndex(const char*
fieldname) {
}
}
-const char* StructSetImpl::getFieldName(int32_t index) {
- for (std::map::iterator iter =
- m_fieldNameIndexMap.begin();
- iter != m_fieldNameIndexMap.end(); ++iter) {
-if (iter->second == index) return iter->first.c_str();
+const std::string& StructSetImpl::getFieldName(int32_t index) {
+ for (const auto& iter : m_fieldNameIndexMap) {
+if (iter.second == index) return (iter.first);
}
- return nullptr;
+
+ throw OutOfRangeException("Struct: fieldName not found.");
--- End diff --
why not `std::out_of_range`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r127278443
--- Diff: src/cppcache/src/StructSetImpl.cpp ---
@@ -66,8 +66,8 @@ SelectResultsIterator StructSetImpl::getIterator() {
return SelectResultsIterator(m_structVector, shared_from_this());
}
-int32_t StructSetImpl::getFieldIndex(const char* fieldname) {
- std::map::iterator iter =
+int32_t StructSetImpl::getFieldIndex(const std::string& fieldname) {
+ const auto& iter =
m_fieldNameIndexMap.find(fieldname);
if (iter != m_fieldNameIndexMap.end()) {
return iter->second;
--- End diff --
We should clear out commented sources.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r127278256
--- Diff: src/cppcache/src/StructSetImpl.cpp ---
@@ -66,8 +66,8 @@ SelectResultsIterator StructSetImpl::getIterator() {
return SelectResultsIterator(m_structVector, shared_from_this());
}
-int32_t StructSetImpl::getFieldIndex(const char* fieldname) {
- std::map::iterator iter =
+int32_t StructSetImpl::getFieldIndex(const std::string& fieldname) {
--- End diff --
should this method be a const method?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #107: GEODE-3019: Refactor Struct class
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/107#discussion_r127312259
--- Diff: src/cppcache/src/StructSetImpl.cpp ---
@@ -78,13 +78,12 @@ int32_t StructSetImpl::getFieldIndex(const char*
fieldname) {
}
}
-const char* StructSetImpl::getFieldName(int32_t index) {
- for (std::map::iterator iter =
- m_fieldNameIndexMap.begin();
- iter != m_fieldNameIndexMap.end(); ++iter) {
-if (iter->second == index) return iter->first.c_str();
+const std::string& StructSetImpl::getFieldName(int32_t index) {
+ for (const auto& iter : m_fieldNameIndexMap) {
+if (iter.second == index) return (iter.first);
}
- return nullptr;
+
+ throw OutOfRangeException("Struct: fieldName not found.");
--- End diff --
I am more keen myself on using `std::exception`s rather than our own.
Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #111: Just a test PR
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/111 Just a test PR Please ignore this while we test PR CI You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native wip/test-pr Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/111.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #111 commit 6cb38062168e5d3ec92c67ea29cf50447e2b2966 Author: Ernie Burghardt Date: 2017-07-24T16:39:19Z Just a test --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #114: GEODE-2729: Remove global variables
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/114 Please fix up commit messages --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/115 GEODE-3165: Reogranized sources relative to the root for better CMake ⦠IDE integration. - Moved src/docs to docs/api. - Moved src/* to root. - Fixup paths in CMake files. You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native feature/GEODE-3165 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/115.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #115 commit 6cbd424fe39bc907025460c555768552430c0c5d Author: Jacob Barrett Date: 2017-08-11T15:30:49Z GEODE-3165: Reogranized sources relative to the root for better CMake IDE integration. - Moved src/docs to docs/api. - Moved src/* to root. - Fixup paths in CMake files. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/115#discussion_r132755738 --- Diff: CMakeLists.txt --- @@ -242,7 +242,7 @@ add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) add_subdirectory(tests) add_subdirectory(templates/security) -add_subdirectory(docs) +add_subdirectory(docs/api) --- End diff -- Could not move /src/docs to /docs since /docs already exists. /src/docs are just the config to generate api docs. So /docs/api made sense. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/115 @dgkimura which CMake find modules are you referring to? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/115#discussion_r132769880 --- Diff: CMakeLists.txt --- @@ -242,7 +242,7 @@ add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) add_subdirectory(tests) add_subdirectory(templates/security) -add_subdirectory(docs) +add_subdirectory(docs/api) --- End diff -- There are API docs and the manual. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/115 @dgkimura Those files actually need to be redone in a way more closely aligned with other CMake find modules. I would leave them alone for now and fix later. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/115 @dgkimura https://issues.apache.org/jira/browse/GEODE-3432 Anything else you don't like? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #114: GEODE-2729: Remove global variables
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/114 This was merged without auto-close comment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #116: GEODE-3439: Split ThinClientRegionInterestTe...
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/116 GEODE-3439: Split ThinClientRegionInterestTestsN into mulitple tests ⦠â¦to avoid random failures. You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native feature/GEODE-3439 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/116.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #116 commit 70d2456b011f52c3c7b77bd362a567d6a0dbfb69 Author: Jacob Barrett Date: 2017-08-14T23:04:43Z GEODE-3439: Split ThinClientRegionInterestTestsN into mulitple tests to avoid random failures. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #387: GEODE-2411: Remove references to Gemfire from include guar...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode/pull/387 The convention in this pull request is not consistent with the Google C++ Style Guide. https://google.github.io/styleguide/cppguide.html#The__define_Guard --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #6: GEODE-2470: Replace sed dependency with standard C++.
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/6 Why not use regex_replace in C++11 http://en.cppreference.com/w/cpp/regex/regex_replace? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #3: Replace ace calls to standard functions
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/3#discussion_r100877742
--- Diff: src/cppcache/src/CacheXmlParser.cpp ---
@@ -1165,7 +1166,7 @@ void CacheXmlParser::startPersistenceManager(const
xmlChar** atts) {
throw CacheXmlException(s.c_str());
}
- ACE_OS::strncpy(libraryFunctionName, (char*)atts[i], len);
+ std::strncpy(libraryFunctionName, (char*)atts[i], len);
--- End diff --
Maybe not is a great opportunity to fix these c-style casts that are
warnings in C++11 now when you are fixing other things in the file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #3: Replace ace calls to standard functions
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/3#discussion_r100878314
--- Diff: src/cppcache/src/TcrMessage.hpp ---
@@ -1170,7 +1170,7 @@ class TcrMessageHelper {
}
if (compId != expectedPartType) {
char exMsg[256];
- ACE_OS::snprintf(exMsg, 255,
+ std::snprintf(exMsg, 255,
--- End diff --
This change violates the C++ formatting we adhere to. Please reformat each
changed line or file after global search and replace.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #3: Replace ace calls to standard functions
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/3#discussion_r100878727
--- Diff: src/cppcache/src/TcrMessage.hpp ---
@@ -1170,7 +1170,7 @@ class TcrMessageHelper {
}
if (compId != expectedPartType) {
char exMsg[256];
- ACE_OS::snprintf(exMsg, 255,
+ std::snprintf(exMsg, 255,
--- End diff --
Technically its the lines following this change that violate the formatting.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #377: GEODE-1435: Adds binary distribution licensing.
Github user pivotal-jbarrett closed the pull request at: https://github.com/apache/geode/pull/377 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #9: [GEODE-2408] Refactor CacheableDate with C++11...
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/9 [GEODE-2408] Refactor CacheableDate with C++11 standards. Moved from old repo to new repo. Can I get a thumbs up please? You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native feature/GEODE-2408 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/9.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #9 commit 1d16b3be249693b7bd5bf4422445a0635c9d1379 Author: Jacob Barrett Date: 2016-12-06T01:52:31Z GEODE-2408: Refactor CacheableDate with C++11 standards. commit bc1a8faccd7dceb2e37abe632ab809adc23e790a Author: Jacob Barrett Date: 2017-02-01T06:15:07Z GEODE-2408: Updated usage of deprecated CacheableDate. commit e1f99a67b77658178f2d5e3b025ed5f1369060ca Author: Jacob Barrett Date: 2017-02-01T06:34:44Z GEODE-2408: Removed non-standard timeval type methods. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode pull request #385: [GEODE-2408] Refactor CacheableDate to use C++ std:...
Github user pivotal-jbarrett closed the pull request at: https://github.com/apache/geode/pull/385 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode issue #385: [GEODE-2408] Refactor CacheableDate to use C++ std::chrono
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode/pull/385 Moved to https://github.com/apache/geode-native/pull/9 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #3: Replace ace calls to standard functions
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/3#discussion_r100912867
--- Diff: src/cppcache/src/TcrMessage.hpp ---
@@ -1170,7 +1170,7 @@ class TcrMessageHelper {
}
if (compId != expectedPartType) {
char exMsg[256];
- ACE_OS::snprintf(exMsg, 255,
+ std::snprintf(exMsg, 255,
--- End diff --
We don't have a good command to execute. I suggest looking for integration
into what ever editor you have with clang-format. There appears to be some
differences between clang-format on mac and windows so limit the formatting to
files you are editing.
For example, in Eclipse, I have reformat on save configured.
The following lines used to be aligned with the changed line's open paren.
The guide defines specific rules for when to align to the paren or indent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #3: Replace ace calls to standard functions
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/3#discussion_r100913452
--- Diff: src/cppcache/src/CacheXmlParser.cpp ---
@@ -1165,7 +1166,7 @@ void CacheXmlParser::startPersistenceManager(const
xmlChar** atts) {
throw CacheXmlException(s.c_str());
}
- ACE_OS::strncpy(libraryFunctionName, (char*)atts[i], len);
+ std::strncpy(libraryFunctionName, (char*)atts[i], len);
--- End diff --
I am of the camp that believes that large efforts like that are rarely
undertaken unless there is an easy tool to do the work for you. The clang-tidy
rule fixed some when we did the initial reformat and cleanup to Google C++
Style Guide. There were some it was unable to do automagically, my guess
because they were ambiguous. I would make an effort that any file you edit
should compile clean, no warnings and no static analyzer warnings/errors. You
can enable clang-tidy by passing CLANG_TIDY_ENABLED=ON to CMake.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #3: Replace ace calls to standard functions
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/3#discussion_r100942381
--- Diff: src/cppcache/src/CacheXmlParser.cpp ---
@@ -1165,7 +1166,7 @@ void CacheXmlParser::startPersistenceManager(const
xmlChar** atts) {
throw CacheXmlException(s.c_str());
}
- ACE_OS::strncpy(libraryFunctionName, (char*)atts[i], len);
+ std::strncpy(libraryFunctionName, (char*)atts[i], len);
--- End diff --
The C++ way to do this now is `reinterpret_cast(atts[i])`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #6: GEODE-2470: Replace sed dependency with standa...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/6#discussion_r100956921
--- Diff: src/cppcache/integration-test/CacheHelper.cpp ---
@@ -1382,6 +1377,31 @@ void
CacheHelper::createDuplicateXMLFile(std::string& originalFile,
CacheHelper::staticConfigFileList.size());
}
+void CacheHelper::replacePortsInFile(int hostPort1, int hostPort2,
+ int hostPort3, int hostPort4, int
locPort1,
+ int locPort2, const std::string&
inFile,
+ const std::string& outFile) {
+ std::ifstream in(inFile, std::ios::in | std::ios::binary);
+ if (in) {
+std::string contents;
+in.seekg(0, std::ios::end);
+contents.resize(in.tellg());
+in.seekg(0, std::ios::beg);
+in.read(&contents[0], contents.size());
--- End diff --
```
contents.assign((std::istreambuf_iterator(in)),
std::istreambuf_iterator());
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #6: GEODE-2470: Replace sed dependency with standa...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/6#discussion_r100956809
--- Diff: src/cppcache/integration-test/CacheHelper.cpp ---
@@ -1382,6 +1377,31 @@ void
CacheHelper::createDuplicateXMLFile(std::string& originalFile,
CacheHelper::staticConfigFileList.size());
}
+void CacheHelper::replacePortsInFile(int hostPort1, int hostPort2,
+ int hostPort3, int hostPort4, int
locPort1,
+ int locPort2, const std::string&
inFile,
+ const std::string& outFile) {
+ std::ifstream in(inFile, std::ios::in | std::ios::binary);
+ if (in) {
+std::string contents;
+in.seekg(0, std::ios::end);
+contents.resize(in.tellg());
--- End diff --
I think you want `reserve()`. `resize()` causes the string to be resized
and initialized with `CharT()`. `reserve()` just allocates enough capacity but
does not resize or initialize the string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #6: GEODE-2470: Replace sed dependency with standa...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/6#discussion_r100957079
--- Diff: src/cppcache/integration-test/CacheHelper.cpp ---
@@ -1382,6 +1377,31 @@ void
CacheHelper::createDuplicateXMLFile(std::string& originalFile,
CacheHelper::staticConfigFileList.size());
}
+void CacheHelper::replacePortsInFile(int hostPort1, int hostPort2,
+ int hostPort3, int hostPort4, int
locPort1,
+ int locPort2, const std::string&
inFile,
+ const std::string& outFile) {
+ std::ifstream in(inFile, std::ios::in | std::ios::binary);
+ if (in) {
+std::string contents;
+in.seekg(0, std::ios::end);
+contents.resize(in.tellg());
+in.seekg(0, std::ios::beg);
+in.read(&contents[0], contents.size());
+in.close();
+
+contents = std::regex_replace(contents, std::regex("HOST_PORT1"),
std::to_string(hostPort1));
+contents = std::regex_replace(contents, std::regex("HOST_PORT2"),
std::to_string(hostPort2));
+contents = std::regex_replace(contents, std::regex("HOST_PORT3"),
std::to_string(hostPort3));
+contents = std::regex_replace(contents, std::regex("HOST_PORT4"),
std::to_string(hostPort4));
+contents = std::regex_replace(contents, std::regex("LOC_PORT1"),
std::to_string(locPort1));
+contents = std::regex_replace(contents, std::regex("LOC_PORT2"),
std::to_string(locPort2));
+
+std::ofstream out(outFile, std::ios::out);
+out.write(&contents[0], contents.size());
--- End diff --
```
out << contents;
out.close();
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #7: Feature/geode 2467
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/7#discussion_r100958307
--- Diff: src/dependencies/xerces-c/CMakeLists.txt ---
@@ -32,10 +32,10 @@ if (WIN32)
set( _BUILD_DIR Build/Win32/VC${MSVC_VERSION}/${_DEBUG_OR_RELEASE} )
endif()
- if (MSVC_VERSION GREATER 12)
-# Only have project files for VS12 and older
-set(MSVC_VERSION 12)
- endiF()
+ #if (MSVC_VERSION GREATER 12)
--- End diff --
xerces-c does not have project files for VC>14. You should limit the
MSVC_VERSION locally in the module to 14 otherwise xerces-c will not compile.
See the `projects\Win32\` directory in the xerces-c distribution for list of
supported VC.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #7: Feature/geode 2467
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/7#discussion_r100958376
--- Diff: src/dependencies/xerces-c/CMakeLists.txt ---
@@ -32,10 +32,10 @@ if (WIN32)
set( _BUILD_DIR Build/Win32/VC${MSVC_VERSION}/${_DEBUG_OR_RELEASE} )
endif()
- if (MSVC_VERSION GREATER 12)
-# Only have project files for VS12 and older
-set(MSVC_VERSION 12)
- endiF()
+ #if (MSVC_VERSION GREATER 12)
--- End diff --
Also, if we are removing code we should remove it, not comment it out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #11: GEODE-2486: Initialize OpenSSL for DEFAULT ci...
GitHub user pivotal-jbarrett opened a pull request: https://github.com/apache/geode-native/pull/11 GEODE-2486: Initialize OpenSSL for DEFAULT cipher support. - Init SSLv23_client mode to support negotiation of all SSL/TLS versions. - Cleanup C++ style issues. - Update tests to use NON-NULL cipher. You can merge this pull request into a Git repository by running: $ git pull https://github.com/pivotal-jbarrett/geode-native feature/GEODE-2486 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/11.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #11 commit ad8b5a83bac8d972d7ad46bbee201b056c1d436d Author: Jacob Barrett Date: 2017-02-15T06:28:05Z GEODE-2486: Initialize OpenSSL for DEFAULT cipher support. - Init SSLv23_client mode to support negotiation of all SSL/TLS versions. - Cleanup C++ style issues. - Update tests to use NON-NULL cipher. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #15: GEODE-2484: Fix snprintf error. Need #include...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/15#discussion_r101610701
--- Diff: src/cppcache/src/TcrMessage.hpp ---
@@ -1114,10 +1116,10 @@ class TcrMessageHelper {
} else if (!isObj) {
// otherwise we're currently always expecting an object
char exMsg[256];
- std::snprintf(exMsg, 255,
-"TcrMessageHelper::readChunkPartHeader: "
-"%s: part is not object",
-methodName);
+ snprintf(exMsg, 255,
--- End diff --
If you had to remove std:: to make this work then it isn't working like you
think it is. Let's figure out why and fix correctly,
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #12: GEODE-2309: Remove or ignore apache-rat flagg...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/12#discussion_r101658884 --- Diff: .ratignore --- @@ -23,10 +23,12 @@ # expect script .*changepasswd$ +.*winrm.cloud-init$ # doxygen .*package-list$ .*testframeworkdox.txt$ +docs --- End diff -- This will match any file with "docs" in it. Seems like a rather greedy regex. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101659693 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -20,7 +20,7 @@ * limitations under the License. */ -#include "gfcpp_globals.hpp" +#include "geode_globals.hpp" #include "gf_types.hpp" --- End diff -- Why not gf_types.hpp renamed to geode_types.hpp? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101659601 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -1,7 +1,7 @@ #pragma once -#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ -#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ +#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ --- End diff -- Really? GEODE_GEODE doesn't seem right?! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101659366 --- Diff: src/CMakeLists.txt --- @@ -224,7 +222,7 @@ add_subdirectory(cppcache) add_subdirectory(cryptoimpl) add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) -add_subdirectory(gfcpp) +add_subdirectory(getversion) --- End diff -- Eeeek! This is the best name we could come up with for this stupid util? May we should just delete it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #15: GEODE-2484: Fix snprintf error. Need #include...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/15#discussion_r101662122
--- Diff: src/cppcache/src/TcrMessage.hpp ---
@@ -1113,12 +1117,11 @@ class TcrMessageHelper {
return NULL_OBJECT;
} else if (!isObj) {
// otherwise we're currently always expecting an object
- char exMsg[256];
- std::snprintf(exMsg, 255,
-"TcrMessageHelper::readChunkPartHeader: "
-"%s: part is not object",
-methodName);
- LOGDEBUG("%s ", exMsg);
+
+ std::stringstream s;
+ s << "TcrMessageHelper::readChunkPartHeader: " << methodName << ":
part is not object\n";
+ LOGDEBUG("%s ", s.str().c_str());
--- End diff --
Good question. I am still fascinated as to why std::snprintf isn't working
correctly on Windows too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #12: GEODE-2309: Remove or ignore apache-rat flagg...
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/12#discussion_r101671763 --- Diff: .ratignore --- @@ -23,10 +23,12 @@ # expect script .*changepasswd$ +.*winrm.cloud-init$ # doxygen .*package-list$ .*testframeworkdox.txt$ +docs --- End diff -- Hmm.. ok. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101824812 --- Diff: src/CMakeLists.txt --- @@ -224,7 +222,7 @@ add_subdirectory(cppcache) add_subdirectory(cryptoimpl) add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) -add_subdirectory(gfcpp) +add_subdirectory(getversion) --- End diff -- If they are relying on it then their scripts will break either way. There are better ways to get version. I don't think I have ever seen anyone anywhere use it. I don't even see it in our documents. I say kill it! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101825168 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -1,7 +1,7 @@ #pragma once -#ifndef GEODE_GFCPP_ATTRIBUTESFACTORY_H_ -#define GEODE_GFCPP_ATTRIBUTESFACTORY_H_ +#ifndef GEODE_GEODE_ATTRIBUTESFACTORY_H_ --- End diff -- I see an argument that `include/geode/` is the root for the public headers, therefore the guards should be `GEODE_ATTRIBUTESFACTORY_H_`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #19: GEODE-2508: Inital work on making lib names g...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/19#discussion_r102271148
--- Diff: src/cppcache/integration-test/CMakeLists.txt ---
@@ -21,7 +21,7 @@ target_link_libraries(${TEST_UTILS_LIB}
PRIVATE
ACE
PUBLIC
-apache-geode
+${PRODUCT_LIB_NAME}
--- End diff --
Rather than changing the target name of the library throughout CMake by
using a variable I suggest just controlling the output name of the binary using
the OUTPUT_NAME property.
set_target_properties(apache-geode PROPERTIES OUTPUT_NAME
${PRODUCT_LIB_NAME})
This way the target stays "apache-geode" but the results of the output will
be adjusted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #24: GEODE-2508: Initial work on new approach to generic ...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/24 @PivotalSarge yes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #24: GEODE-2508: Initial work on new approach to g...
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/24#discussion_r102517638
--- Diff: src/cppcache/src/CMakeLists.txt ---
@@ -125,7 +125,7 @@ target_include_directories(apache-geode
$
)
add_dependencies(client-libraries apache-geode)
-set_target_properties(apache-geode PROPERTIES PUBLIC_HEADER
"${PUBLIC_HEADERS}")
+set_target_properties(apache-geode PROPERTIES PUBLIC_HEADER
"${PUBLIC_HEADERS}" OUTPUT_NAME ${PRODUCT_LIB_NAME} )
--- End diff --
I would only suggest avoiding long lines in the CMake files by putting each
property on a new line.
```
set_target_properties(apache-geode PROPERTIES
PUBLIC_HEADER "${PUBLIC_HEADERS}"
OUTPUT_NAME ${PRODUCT_LIB_NAME})
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #17: GEODE-2440: Switch hashcode() return type.
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/17 Please close this pull request and resubmit when you have cleaned up all the places that use hashcode. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #25: GEODE-2382: Replace gemstone with geode.
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/25#discussion_r102736948 --- Diff: src/xsds/gfcpp-cache-9.0.xsd --- @@ -33,7 +33,7 @@ limitations under the License. version="9.0"> The contents of a declarative XML file correspond to APIs found in the -com.gemstone.gemfire.cache and com.gemstone.gemfire.cache.client +org.apache.geode.cache and org.apache.geode.cache.client --- End diff -- Have you verified that these are the correct packages after GemFire became Geode? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/27#discussion_r102739212 --- Diff: src/clicache/src/CacheableDate.cpp --- @@ -90,7 +90,7 @@ namespace Apache TimeSpan epochSpan = m_dateTime - EpochTime; int64_t millitime = epochSpan.Ticks / TimeSpan::TicksPerMillisecond; - m_hashcode = (int)millitime ^ (int)((int64)millitime >> 32); + m_hashcode = (int)millitime ^ (int)((int64_t)millitime >> 32); --- End diff -- C++/CLI code does not need to worry about c-style casting since it is also not C++11. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102745344
--- Diff: src/cppcache/src/statistics/HostStatHelperSolaris.cpp ---
@@ -86,23 +86,23 @@ void
HostStatHelperSolaris::refreshProcess(ProcessStats* processStats) {
}
close(fPtr);
}
- ACE_OS::snprintf(procFileName, 64, "/proc/%u/usage", (uint32)thePid);
+ ACE_OS::snprintf(procFileName, 64, "/proc/%u/usage", (uint32_t)thePid);
--- End diff --
Replace c-style casting where you are touching to C++11 style.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102742460
--- Diff:
src/cppcache/integration-test/testThinClientFixedPartitionResolver.cpp ---
@@ -245,7 +245,7 @@ DUNIT_TASK_DEFINITION(CLIENT1,
CheckPrSingleHopForIntKeysTask_REGION)
if (networkhop) {
failureCount++;
}
-int8 serverGroupFlag =
TestUtils::getCacheImpl(getHelper()->cachePtr)
+int8_t serverGroupFlag =
TestUtils::getCacheImpl(getHelper()->cachePtr)
--- End diff --
This sounds like another `bool`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102743540
--- Diff: src/cppcache/src/CacheImpl.cpp ---
@@ -185,8 +185,8 @@ bool CacheImpl::getAndResetNetworkHopFlag() {
return networkhop;
}
-int8 CacheImpl::getAndResetServerGroupFlag() {
- int8 serverGroupFlag = CacheImpl::s_serverGroupFlag;
+int8_t CacheImpl::getAndResetServerGroupFlag() {
+ int8_t serverGroupFlag = CacheImpl::s_serverGroupFlag;
--- End diff --
Is this really `bool`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102744883
--- Diff: src/cppcache/src/statistics/GeodeStatisticsFactory.hpp ---
@@ -98,50 +98,50 @@ class GeodeStatisticsFactory : public StatisticsFactory
{
Statistics* createStatistics(StatisticsType* type, const char* textId);
Statistics* createStatistics(StatisticsType* type, const char* textId,
- int64 numericId);
+ int64_t numericId);
Statistics* createOsStatistics(StatisticsType* type, const char* textId,
- int64 numericId);
+ int64_t numericId);
Statistics* createAtomicStatistics(StatisticsType* type);
Statistics* createAtomicStatistics(StatisticsType* type, const char*
textId);
Statistics* createAtomicStatistics(StatisticsType* type, const char*
textId,
- int64 numericId);
+ int64_t numericId);
// StatisticsFactory methods: Statistics Type
//--
StatisticsType* createType(const char* name, const char* description,
- StatisticDescriptor** stats, int32
statsLength);
+ StatisticDescriptor** stats, int32_t
statsLength);
StatisticsType* findType(const char* name);
// StatisticsFactory methods: Statistics Descriptor
//-
StatisticDescriptor* createIntCounter(const char* name,
const char* description,
-const char* units, int8
largerBetter);
+const char* units, int8_t
largerBetter);
--- End diff --
Pretty sure these are `bool`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/27#discussion_r102738544 --- Diff: src/clicache/src/CacheableDate.cpp --- @@ -90,7 +90,7 @@ namespace Apache TimeSpan epochSpan = m_dateTime - EpochTime; int64_t millitime = epochSpan.Ticks / TimeSpan::TicksPerMillisecond; - m_hashcode = (int)millitime ^ (int)((int64)millitime >> 32); + m_hashcode = (int)millitime ^ (int)((int64_t)millitime >> 32); --- End diff -- You should also be addressing: - Thec-style casting - Use of int vs int32_t ``` int64_t millitime = epochSpan.Ticks / TimeSpan::TicksPerMillisecond; m_hashcode = static_cast(millitime) ^ static_cast(millitime >> 32); ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102743673
--- Diff: src/cppcache/src/CqServiceVsdStats.hpp ---
@@ -108,7 +108,7 @@ class CPPCACHE_EXPORT CqServiceVsdStats : public
CqServiceStatistics {
class CqServiceStatType {
private:
- static int8 instanceFlag;
+ static int8_t instanceFlag;
--- End diff --
`bool`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/27#discussion_r102739037 --- Diff: src/clicache/src/CacheableDate.cpp --- @@ -90,7 +90,7 @@ namespace Apache TimeSpan epochSpan = m_dateTime - EpochTime; int64_t millitime = epochSpan.Ticks / TimeSpan::TicksPerMillisecond; - m_hashcode = (int)millitime ^ (int)((int64)millitime >> 32); + m_hashcode = (int)millitime ^ (int)((int64_t)millitime >> 32); --- End diff -- Actually, I question the need to convert any of the C++/CLI sources since the C++/CLI language isn't C++ and defines its own integer types. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102747392
--- Diff: src/cppcache/src/statistics/StatisticDescriptorImpl.cpp ---
@@ -41,8 +41,8 @@ StatisticDescriptorImpl::StatisticDescriptorImpl(const
char* statName,
FieldType
statDescriptorType,
const char*
statDescription,
const char* statUnit,
- int8 statIsStatCounter,
- int8
statIsStatLargerBetter) {
+ int8_t statIsStatCounter,
--- End diff --
`bool`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102743360
--- Diff: src/cppcache/src/BucketServerLocation.hpp ---
@@ -130,7 +130,7 @@ class BucketServerLocation : public ServerLocation {
}
uint32_t objectSize() const {
-return sizeof(int) + sizeof(bool) + sizeof(int8);
+return sizeof(int) + sizeof(bool) + sizeof(int8_t);
--- End diff --
For completeness `int` should be changed `int32_t`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102746696
--- Diff: src/cppcache/src/statistics/HostStatHelperWin.cpp ---
@@ -535,21 +535,21 @@ uint32
HostStatHelperWin::getInt32Value(PPERF_COUNTER_DEFINITION PerfCntr,
}
if (PerfCntr->CounterSize == 4) {
-uint32* lptr;
-lptr = (uint32*)((char*)PerfCntrBlk + PerfCntr->CounterOffset);
+uint32_t* lptr;
+lptr = (uint32_t*)((char*)PerfCntrBlk + PerfCntr->CounterOffset);
--- End diff --
What is this really trying to do? Maybe fix it?
C++11 style casting changes at least.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102742228
--- Diff: src/cppcache/include/geode/statistics/StatisticsFactory.hpp ---
@@ -96,7 +96,7 @@ class CPPCACHE_EXPORT StatisticsFactory {
virtual StatisticDescriptor* createDoubleCounter(
const char* name, const char* description, const char* units,
- int8 largerBetter = true) = 0;
+ int8_t largerBetter = true) = 0;
--- End diff --
Might be an opportunity to correct this to `bool`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/27#discussion_r102739522 --- Diff: src/cppcache/include/geode/geode_base.hpp --- @@ -69,32 +69,9 @@ #define GF_TEMPLATE_EXPORT #endif -#if defined(_MSC_VER) -/* 32 bit Windows only, for now */ -typedef signed char int8;/**< single byte, character or boolean field */ -typedef unsigned char uint8; /**< unsigned integer value */ -typedef signed short int16; /**< signed 16 bit integer (short) */ -typedef unsigned short uint16; /**< unsigned 16 bit integer (ushort) */ -typedef signed int int32;/**< signed 32 bit integer */ -typedef unsigned int uint32; /**< unsigned 32 bit integer */ -typedef signed __int64 int64;/**< signed 64 bit integer */ -typedef unsigned __int64 uint64; /**< unsigned 64 bit integer */ - -// typedef int32intptr_t; /**< a pointer to a 32 bit integer */ -// typedef uint32 uintptr_t; /**< a pointer to an unsigned 32 bit -// integer */ - -/* Windows does not have stdint.h */ -typedef int8 int8_t; -typedef uint8 uint8_t; -typedef int16 int16_t; -typedef uint16 uint16_t; -typedef int32 int32_t; -typedef uint32 uint32_t; -typedef int64 int64_t; -typedef uint64 uint64_t; -/* end stdint.h */ +#include --- End diff -- Is it really necessary to include this at such a low level? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r10278
--- Diff: src/cppcache/src/RegionStats.hpp ---
@@ -129,7 +129,7 @@ class CPPCACHE_EXPORT RegionStats {
class RegionStatType {
private:
- static int8 instanceFlag;
+ static int8_t instanceFlag;
--- End diff --
`bool`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #28: Feature/geode 2440
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/28#discussion_r102749040
--- Diff: src/cppcache/src/ClientMetadata.cpp ---
@@ -344,7 +344,7 @@ int ClientMetadata::assignFixedBucketId(const char*
partitionName,
FixedMapType::iterator iter = m_fpaMap.find(partitionName);
if (iter != m_fpaMap.end()) {
std::vector attList = iter->second;
-int hc = static_cast(resolvekey->hashcode());
+int hc = resolvekey->hashcode();
--- End diff --
Use consistent typing. Change `int` to `int32_t`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request: https://github.com/apache/geode-native/pull/27#discussion_r102789698 --- Diff: src/clicache/src/CacheableDate.cpp --- @@ -90,7 +90,7 @@ namespace Apache TimeSpan epochSpan = m_dateTime - EpochTime; int64_t millitime = epochSpan.Ticks / TimeSpan::TicksPerMillisecond; - m_hashcode = (int)millitime ^ (int)((int64)millitime >> 32); + m_hashcode = (int)millitime ^ (int)((int64_t)millitime >> 32); --- End diff -- This version of CacheableDate is C++/CLI. C++/CLI is only C++ in name. It does not make sense to use C++11 types in C++/CLI despite the fact that they "work". In C++/CLI `int`, `long`, etc. are not ambiguous like they are in C++. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native issue #30: GEODE-2532: Create config files in runtime directory...
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/30 Does this change create the xml files in each of the individual integration test's runtime directory of the root of the all the tests? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102813470
--- Diff: src/cppcache/src/RegionStats.hpp ---
@@ -129,7 +129,7 @@ class CPPCACHE_EXPORT RegionStats {
class RegionStatType {
private:
- static int8 instanceFlag;
+ static int8_t instanceFlag;
--- End diff --
Turns out all this instanceFlag variables are unused. Please delete them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on a diff in the pull request:
https://github.com/apache/geode-native/pull/27#discussion_r102813688
--- Diff:
src/cppcache/integration-test/testThinClientFixedPartitionResolver.cpp ---
@@ -245,7 +245,7 @@ DUNIT_TASK_DEFINITION(CLIENT1,
CheckPrSingleHopForIntKeysTask_REGION)
if (networkhop) {
failureCount++;
}
-int8 serverGroupFlag =
TestUtils::getCacheImpl(getHelper()->cachePtr)
+int8_t serverGroupFlag =
TestUtils::getCacheImpl(getHelper()->cachePtr)
--- End diff --
I looked into the source, serverGroupFlag is not boolean value but rather
an enum.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #27: GEODE-2439: Remove non-standard int typedefs
Github user pivotal-jbarrett commented on the issue: https://github.com/apache/geode-native/pull/27 A rebase would have been preferable to a merge. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
