remove flux_msg_handler_t member from struct flux_msg_handler_spec

[morrone]

The flux_msg_handler_t pointer in struct flux_msg_handler_spec (in src/common/libflux/dispatch.h) seems a little odd to me. I can only guess that the intent was to allow bulk registration of handlers, but still allow the code to individually stop and restart the handler later?

That seems a little a pretty unlikely use case to me. It also turns a simple one-way passing of information into a potentially slightly more complicated two-way passing of information using the same array.

A survey of the code shows that nowhere is this used in either flux core or flux sched.

I am going to suggest that if an application needs to stop and start a registered handler, it can just use the non-array form of the create/start/stop/destroy functions rather than the array form.

I propose that we simplify the struct flux_msg_handler_spec to eliminate the flux_msg_handler_t field.

[garlick]

Actually, it's used by flux_msg_handler_delvec() to destroy the handlers created by _addvec().

[morrone]

Ahhh, I see. I would propose that the flux_msg_handler_t array be broken out from the flux_msg_handler_spec. I image that flux_msg_handler_delvec() ignores everything but the flux_msg_handler_t field, which makes it a little strange to pass it all of that information. By breaking it apart, we have clear input and output variables.

So this:

struct flux_msg_handler_spec {
    int typemask;
    char *topic_glob;
    flux_msg_handler_f cb;
    uint32_t rolemask;
    flux_msg_handler_t *w;
};
#define FLUX_MSGHANDLER_TABLE_END { 0, NULL, NULL, 0, NULL }

int flux_msg_handler_addvec (flux_t *h, struct flux_msg_handler_spec tab[],
                             void *arg);
void flux_msg_handler_delvec (struct flux_msg_handler_spec tab[]);

would become something like this:

struct flux_msg_handler_spec {
    int typemask;
    char *topic_glob;
    flux_msg_handler_f cb;
    uint32_t rolemask;
};
#define FLUX_MSGHANDLER_TABLE_END { 0, NULL, NULL, 0, NULL }

int flux_msg_handler_addvec (flux_t *h, void *arg, const struct flux_msg_handler_spec tab[],
                                                  flux_msg_handler_t **msg_handlers);
void flux_msg_handler_delvec (struct flux_msg_handler_t **msg_handlers);

[garlick]

That sounds like a good cleanup. delvec would need a count argument too I guess...