I've looked through this a bit, but I'll need a single patch file before I can start testing things. Also, there's not much in the way of comments; can you walk us through how all of the pieces fit together?
A couple of things that stand out:
- Can you verify that this patch works on the latest 1.5.0 release? We made some changes to the avro_datum_t API (
AVRO-751), but it looks like you're using the old 1.4.x API.
- You include memory.h, but I think that with the modern C standards that header is deprecated in favor of string.h. string.h gives you all of the mem* functions that you're using.
- You also have to be careful about which Avro headers you're using in your generated code. The only headers that are installed are avro.h, and the header files in src/avro. The others are only available while compiling the Avro library itself.
- What platform are you testing on? I think your semaphore implementation (sem_t) won't work on Mac OS X. But that's nicely segregated, so hopefully we can drop something else in pretty easily.
- An aesthetic point — I'm not sure how picky we're being about formatting guidelines, but the rest of the Avro C library uses the Linux formatting conventions: indent with actual tab characters, tab width is 8 spaces. (It's not my favorite, but I've been aiming for consistency when writing my own patches...)
- The fact that you're using pthreads might make this not work on Windows machines. Bruce has more experience on the Win32 side of things, so he can hopefully fill us in on that.
I hope that list doesn't seem too pessimistic — I'm glad to see this contribution, too!
My last concern is with the code generation. Can you describe what's being produced by the velocity templates? Can you walk through which parts are generated code, and which parts are provided by the Avro library itself?
I have a set of patches I've been working on for generating custom C types for Avro schemas (AVRO-778), so I'd like to make sure that there isn't too much overlap. My schema compiler doesn't use Velocity, though; it's written in C. I did this b/c I don't want to require users of the C library to install the Java library as well, just to be able to do code generation.