Just want to +1 on the use of a dynamic library - this really has to be a shared lib, interop with most other languages demands it. On the other hand, I'm not a huge fan of making this a separate library from the native client itself, simply because proliferation of binary files makes life difficult for our users. native client on Windows already uses a mixed-mode assembly to avoid having separate C++ and .net DLLs, *and* we're potentially lugging around cryptoimpl with us if someone wishes to use SSL. Adding another library just for C bindings makes for a clean separation of the APIs, but I'm not certain how much value this adds, and it definitely adds complexity to any installation/setup.
I've experienced firsthand the problems of exceptions thrown (or attempting to be thrown, anyway) across the interface boundary. It doesn't work, to put it mildly, so for sure we'll have to address the issue. I've updated the doc to reflect the latest thinking based on this conversation. Let me know if you still think we need more. Thanks, Blake On Mon, Mar 30, 2020 at 7:01 AM Jacob Barrett <jbarr...@pivotal.io> wrote: > > > > On Mar 27, 2020, at 4:04 PM, Matthew Reddington <mredding...@pivotal.io> > wrote: > > * C does not have namespaces or overloading, so we will need a naming > convention to differentiate our types and methods from any other library or > the application code. That means all types and functions should be > prepended with “geode_” or something similar. > > Ha… I knew there was something on my list I forgot to mention. Yes, all > methods need to be prefixed. I recall there being something around Apache > branding that required us to use apache::geode:: as our base namespace > rather than just geode::. Same would hold true for this prefix also then. I > would suggest apache_geode_X. The method names get pretty long so it may be > worth double checking that geode_ is sufficient. > > > * We must absolutely produce a dynamic library. Not all FFI’s support > static linking. > > Yeah, I have changed my mind on that. Ideally we would produce both but > dynamic only is fine for now. Given the source release nature of Apache > anyway this doesn’t really matter. Anyone could pull the source and build a > static lib as needed. > > > * I recommend we avoid introducing real types in the exported interface. > In order to support future revision, we’d have to introduce version and > size information into the struct a la THE WIN32 API. Remember that? Having > to zero out the struct then setting the size and version members before the > other members? This is why they did that. > > What is this in reference to? Using structs to hold the pointers? I > already over that solution for just using opaque struct pointers. > > > * The implementation needs to guard against throwing exceptions across > the library boundary. > > I was wondering about this. It feels like all these wrappers should catch > exceptions and convert them to a known set of int error codes, which are > either returned or passed by reference on the functions. None of that makes > for really clean readable C code but then again its C. > > > * This library needs a thread safe means of error handling. There is not > enough fleshed out in this RFC to have a meaningful conversation about this > at this time. > > Can you elaborate on this? If the functions all returned some error code > what thread safety issues are you thinking about? If we need to get more > detail from the exception back to the caller we could use thread local > storage for message string, stack, etc. Perhaps taking some lessons from > JNI and having a few exception checking/clearing/description methods with > thread safe storage. > > > * I would like to see an ability for the client to specify their own > allocators. This would require an overhaul of the existing C++ ABI and may > have an impact on our dependencies. This is another talk for a more > complete RFC. > > I think this is out of scope for sure. As you state the current C++ API > lacks this ability as well as all the internals. Future us issues. > > -Jake > >
