Re: [SSSD] [PATCHES] sbus: support multiple interfaces on single object path

[Pavel Březina]

On 01/16/2015 02:06 PM, Jakub Hrozek wrote:
> > +#define CHECK_SIGNATURE(req, error, label, exp) do { \
> > const char *__sig; \
> > __sig = dbus_message_get_signature(req->message); \
> > if (strcmp(__sig, exp) != 0) { \
> >    error = sbus_error_new(req, DBUS_ERROR_INVALID_ARGS, \
> >           "Invalid arguments: expected \"%s\", got \"%s\"", exp, __sig); \
> >    goto label; \
>
> Another flow-changing macro...I'm fine with wrappering this in a
> function, but please don't jump from macros..

It's funny that you don't like it, since I learned it from you :-)

The whole point of those macros is to move error checking out of the 
function. If you put the code inside a function you still have to test 
the result in the caller so it doesn't make any sense.

I can get rid of CHECK_SIGNATURE since it is used on three places only 
anyway. But WRITE_DATA makes the code more readable. As you said 
yourself, fprintf is not really expected to fail. This way it is 
possible to focus on the flow of the generator itself instead of seeing 
four lines that are not supposed to be executed ever after each fprintf.

This is IMHO much better:
WRITE_DATA(file, ret, done, FMT_METHOD_NOARG, method->name);
WRITE_DATA(file, ret, done, FMT_METHOD, method->name);
WRITE_DATA(file, ret, done, FMT_METHOD_CLOSE);

instead of (copied from expanded macro):
     ret = fprintf(file, "    <method name="%s" />\n", method->name);
     if (ret < 0) {
         ret = 5;
         goto done;
     }

ret = fprintf(file, "    <method name="%s">\n", method->name);
     if (ret < 0) {
         ret = 5;
         goto done;
     }

ret = fprintf(file, "    </method>\n");
     if (ret < 0) {
         ret = 5;
         goto done;
     }