[
https://issues.apache.org/jira/browse/GEODE-10074?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17497022#comment-17497022
]
ASF GitHub Bot commented on GEODE-10074:
----------------------------------------
pivotal-jbarrett commented on a change in pull request #928:
URL: https://github.com/apache/geode-native/pull/928#discussion_r813235302
##########
File path: cppcache/integration-test/CMakeLists.txt
##########
@@ -69,6 +67,10 @@ add_clangformat(unit_test_callbacks)
enable_testing()
+if(UNIX AND NOT APPLE)
Review comment:
This seems a little wonky... Is this perhaps a Linux or GCC specific
dependency? It is also likely that our dependency or `rt` comes from another
library and if so that is where this dependency should be added. Also, see root
CMakeLists.txt for some of these more platform specific dependencies.
##########
File path: cppcache/CMakeLists.txt
##########
@@ -89,7 +89,6 @@ elseif (WIN32)
endif()
target_link_libraries(_apache-geode INTERFACE
- ACE::ACE
Review comment:
Tears of joy!
##########
File path: cppcache/integration-test/fw_helper.hpp
##########
@@ -107,7 +105,7 @@ BEGIN_TEST.
#define ASSERT_EQ(x, y) _ASSERT_EQ(x, y, __LINE__, __FILE__)
#define ASSERT_STREQ(x, y) \
- ASSERT((strcmp(x, y) == 0), \
+ ASSERT(x == y, \
Review comment:
I would be a little hesitant to change the expected type of x and y from
c-string to `std::string`, or really anything with `==` operator overload.
What about `std::strcmp`?
##########
File path: cppcache/src/CppCacheLibrary.cpp
##########
@@ -36,18 +33,15 @@ namespace apache {
namespace geode {
namespace client {
-void CppCacheLibrary::initLib(void) { ACE::init(); }
+void CppCacheLibrary::initLib(void) {}
-void CppCacheLibrary::closeLib(void) {
- // DO NOT CALL ACE::fini() HERE!
- // Things might be using ace beyond the life of Geode.
-}
+void CppCacheLibrary::closeLib(void) {}
std::string CppCacheLibrary::initProductLibDir() {
// otherwise... get the DLL path, and work backwards from it.
- char buffer[PATH_MAX + 1];
+ char buffer[4097];
buffer[0] = '\0';
- DllMainGetPath(buffer, PATH_MAX);
+ DllMainGetPath(buffer, 4096);
Review comment:
use `sizeof(buffer)` to keep them in sync.
##########
File path: cppcache/src/CppCacheLibrary.cpp
##########
@@ -36,18 +33,15 @@ namespace apache {
namespace geode {
namespace client {
-void CppCacheLibrary::initLib(void) { ACE::init(); }
+void CppCacheLibrary::initLib(void) {}
-void CppCacheLibrary::closeLib(void) {
- // DO NOT CALL ACE::fini() HERE!
- // Things might be using ace beyond the life of Geode.
-}
+void CppCacheLibrary::closeLib(void) {}
std::string CppCacheLibrary::initProductLibDir() {
// otherwise... get the DLL path, and work backwards from it.
- char buffer[PATH_MAX + 1];
+ char buffer[4097];
Review comment:
How did you arrive at 4097?
Perhaps refactoring these implementations of `DllMainGetPath` to allocate
and return appropriately sized `std::string` values?
For Windows the old `MAX_PATH`, 260, limit is gone and
[`GetModulFileName`](https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-getmodulefilenamea#return-value)
will set an error condition if the buffer it was given was too small. We could
increase he buffer as needed to get the full path and the initialize a
`std::string` from that buffer before deallocating it.
For POSIX the
[`realpath`](https://pubs.opengroup.org/onlinepubs/009696799/functions/realpath.html)
method can allocate the buffer for us by using `NULL` for the `resolved_path`
parameter, we just need to free it after initializing an `std:string` with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Remove ACE remains
> ------------------
>
> Key: GEODE-10074
> URL: https://issues.apache.org/jira/browse/GEODE-10074
> Project: Geode
> Issue Type: Improvement
> Components: native client
> Reporter: Mario Salazar de Torres
> Assignee: Mario Salazar de Torres
> Priority: Major
> Labels: obliterate-ace, pull-request-available
>
> *AS A* geode-native contributor
> *I WANT TO* remove all the ACE remaining
> *SO THAT* geode-native does not depend on ACE anymore
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
