Bug#755212: transition: protobuf-c

[Gergely Nagy <algernon@balabit.hu>]

Robert Edmonds <edmonds@debian.org> writes:

>> The various riemann-c-client headers in /usr/include/riemann include
>> proto/riemann.pb-c.h, and there's syslog-ng-mod-riemann (from
>> syslog-ng-incubator) that uses the library, thus, the generated header
>> too, transitively.
>
> Ah, right.  From a brief look at the source code for that module it
> looks like it doesn't require a (bin-)NMU at all, if I'm understanding
> the libriemann-client API correctly.

Yep, I agree. syslog-ng-incubator should not need a bin-NMU, as it uses
everything through the libriemann-client API, and never touches the
protobuf structures directly.

>> >     I would recommend that the upstream developers ship a .proto file
>> >     instead.
>> 
>> I'd rather not ship a .proto file, if at all possible. I'll see if I can
>> hide it completely.
>
> This would eliminate the problem, too.
>
> It looks like you typedef the structures generated by protoc-c and wrap
> them in your own API, e.g. from <riemann/query.h>:
>
>     #include <riemann/proto/riemann.pb-c.h>
>
>     typedef Query riemann_query_t;
>
>     riemann_query_t *riemann_query_new (const char *string);
>     void riemann_query_free (riemann_query_t *query);
>
>     int riemann_query_set_string (riemann_query_t *query, const char *string);
>
> ("Query" is from "typedef struct _Query Query" in riemann.pb-c.h.)
>
> If your API callers always use the *_new(), *_free(), etc. functions and
> never try to dereference or calculate sizeof() on the wrapped struct's
> it might be possible to remove the #include of the .pb-c.h file and
> change your typedef to, e.g.:
>
>     typedef struct _Query riemann_query_t;
>
> And then have "riemann_query_t" be an opaque type.  Though this depends
> on protoc-c continuing to generate structure tags with leading
> underscores, which may not always be the case.  (I've wanted to get rid
> of the leading underscores for a while now.)
>
> (Similiarly for the other riemann_*_t types that wrap protoc-c generated
> structures.)
>
> It might also be possible to wrap the structure types generated by
> protoc-c in your own opaque structure type and expose that wrapper type
> via your API.  Something like:
>
>     typedef struct riemann_query riemann_query_t;
>
>     riemann_query_t *riemann_query_new (const char *string);
>     void riemann_query_free (riemann_query_t *query);
>
>     int riemann_query_set_string (riemann_query_t *query, const char *string);
>
> (In <riemann/query.h>.)
>
>     #include "proto/riemann.pb-c.h"
>
>     struct riemann_query {
>         Query query;
>     };
>
>     /* rest of the implementation... */
>
> (In lib/riemann/query.c.)
>
> That's a bit uglier since you have to update accesses to go via the
> wrapper but would provide the maximum amount of insulation between the
> libriemann-client API and the underlying structures generated by the
> protoc-c code generator.

I gave this some more thought, and there's a problem: while generating
riemann events and similar can be done with opaque types, if I do a
query, then I want to access the results, and to do that with opaque
types would mean I need a lot of getter functions (and an API + ABI
bump). So I'll stick to how it is done today, for the foreseeable
future.

But I'll keep your suggestions in mind in case I end up writing another
library that uses protobuf, I'll hide the protobuf stuff deeper then! :)

-- 
|8]