Skip to content

Conversation

@pdxcodemonkey
Copy link
Contributor

@pdxcodemonkey pdxcodemonkey commented Jun 15, 2021

This is the initial work to create a C API for geode-native. The goal of this work is not to enable access to geode-native from the C language, though that will happen as a matter of course, but rather to enable access from all the other languages (.net core, Python, Go, etc.) from which access to a C API is simple and straightforward.

RFC

@pdxcodemonkey pdxcodemonkey marked this pull request as draft June 15, 2021 17:29
Blake Bender and others added 12 commits June 15, 2021 11:45
WIP: Add cache creation, destruction via wrapper

WIP: Add wrapper class for Cache
- implement getPdxReadSerialized, getPdxIgnoreUnreadFields

WIP: Wrappers for PoolManager, PoolFactory, Pool, and RegionFactory
- only bare-bones functionality implemented

WIP: Implement RegionWrapper and RegionFactoryWrapper::createRegion

WIP: String put/get works with Python tests now

Add remove, containsValueForKey to region wrapper

WIP: Add proper CacheFactory wrapper

WIP: getCredentials is working(!)

AuthInitialize wrapper implemented - getCredentials now working from Python

Fix export function declarations on Linux

Add CreateCacheFactory back to header
- Accidentally removed

Co-authored-by: Michael Oleske <moleske@pivotal.io>

WIP: Implement a few more Cache APIs
- added getName, close, isClosed
- switched output to NC logger for debugging traces

WIP: leak checking signs of life

Fix crash in Client wrapper (check for null)

GEODE-7928: C bindings are prefixed with geode_.

Replaced void pointers with opaque pointers for increased type safety. Fixed broken signatures. Removed erroneous and unused implementations.

Comment out throw in geode_ClientUninitialize
- also some formatting changes

[#172065949]: Moved C bindings to it's own top level directory aptly named c_bindings.

Changed prefix from geode_ to apache_geode_.

Reorganized implementation.

Shuffled files. Unnecessary symbols stripped from library.

Fix exports for C bindings on Windows

Added OSX guard around symbol whitelist.

Installing C headers.

Make static lib code position-independent (-fPIC), so that it can be consumed when
building a shared library.

Hiding symbols on Linux.

Add support for Put/GetByteArray

Unit tests. Removed Client singleton.

C++17 nested namespaces, because we deserve it.

Remove extraneous files that got inadvertently checked in
- most likely generated by an automated tool or IDE someone was using
- These are causing RAT check to fail, because they don't have Apache License notices
- include objbase.h to get def for CoTaskMemAlloc, required for .net
  p/invokes
- Insert parens around name of std::numeric_limits::max, to fix clash
  with insane Windows max macro
- Copy apache-geode.dll to output dir of C bindings test
- Revert to C++ 11 standard until we can fix templates in functional.hpp
- bad cast to CacheFactory, rather than CacheFactoryWrapper, was blowing
  up test
- Added new test using C bindings
- Was crashing in leak detection code.  Fixed cause of crash, and also added a catchall block, so we don't throw across the library boundary.
- VS2019 compiler flagged a new warning as an error
- Also reformat C Bindings code
@pdxcodemonkey pdxcodemonkey changed the title C bindings GEODE-9358: Initial revision of C bindings Jun 15, 2021
@mmartell mmartell self-requested a review June 15, 2021 21:41
Copy link
Contributor

@mmartell mmartell left a comment

Choose a reason for hiding this comment

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

Awesome addition to geode-native. With C's stable ABI, seems like the right way to open the door for other languages.

#endif
if (bytes) {
memcpy(byteArray, bytes->value().data(), valSize);
*value = (char*)byteArray;

Choose a reason for hiding this comment

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

There's your clang-tidy problem, use a reinterpret_cast.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, thx.

@pdxcodemonkey pdxcodemonkey marked this pull request as ready for review June 16, 2021 18:36
Copy link

@echobravopapa echobravopapa left a comment

Choose a reason for hiding this comment

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

👍 👍

@echobravopapa echobravopapa merged commit d45a0c8 into apache:develop Jun 17, 2021
@pdxcodemonkey pdxcodemonkey deleted the c_bindings branch June 17, 2021 18:10
davebarnes97 pushed a commit to davebarnes97/geode-native that referenced this pull request Feb 9, 2022
Co-authored-by: Blake Bender <bblake@vmware.com>
Co-authored-by: Matthew Reddington <mreddington@pivotal.io>
Co-authored-by: Michael Oleske <moleske@pivotal.io>
davebarnes97 pushed a commit to davebarnes97/geode-native that referenced this pull request Feb 9, 2022
Co-authored-by: Blake Bender <bblake@vmware.com>
Co-authored-by: Matthew Reddington <mreddington@pivotal.io>
Co-authored-by: Michael Oleske <moleske@pivotal.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants