>>>Does it make sense to have the CacheFactory concept at all (especially since it is more of a builder)? Could we have some C struct that can be used to create the cache, where the struct has fields for all the configuration? In general can we rethink the API so that it makes sense for C or other language bindings? >> * 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.
Jake, you mentioned passing structs to initialize the cache instead of using a cache factory. I'm leery of introducing additional concrete types to yet another API. Since this isn't where we are slow, flexibility and future proofing are preferred. This is something the team is going to have to look at more options, good examples, and decide what we're willing to deal with long term. >> * 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? When I think of C and error handling, I think of errno, which isn't thread safe. I don't want to see a repeat of that. I suppose it will boil down to output parameters and returning error codes. On Mon, Mar 30, 2020 at 10:39 AM Blake Bender <bben...@pivotal.io> wrote: > 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 > > > > >
