Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[minor cleanup]: Use different variable for flux_msg_handler_t pointers #1219

Closed
chu11 opened this issue Oct 3, 2017 · 6 comments
Closed

Comments

@chu11
Copy link
Member

chu11 commented Oct 3, 2017

I got confused looking at some code in msg_handler.c and reactor.c until I realized w is used for flux_msg_handler_t pointers and also for flux_watcher_t pointers.

Looking back, flux_msg_handler_t was once flux_msg_watcher_t, thus the legacy naming. We could update it.

@garlick
Copy link
Member

garlick commented Oct 3, 2017

Works for me. Sorry for my laziness.

@chu11
Copy link
Member Author

chu11 commented Oct 4, 2017

It's worth mentioning if struct flux_msg_handler_spec is updated to change w to something else, that'll incur changes in flux sched as well.

@garlick
Copy link
Member

garlick commented Oct 4, 2017

The common use case is to statically initialize

static struct flux_msg_handler_spec sim_htab[] = {
    {FLUX_MSGTYPE_EVENT, "sim.start", start_cb, 0, NULL},
    {FLUX_MSGTYPE_REQUEST, "sched.trigger", trigger_cb, 0, NULL},
    {FLUX_MSGTYPE_EVENT, "sched.res.*", sim_res_event_cb, 0, NULL},
    FLUX_MSGHANDLER_TABLE_END,
};

and then pass to addvec/delvec

   if (flux_msg_handler_addvec (ctx->h, sim_htab, (void *)h) < 0) {
        flux_log (ctx->h, LOG_ERR, "flux_msg_handler_addvec: %s", strerror (errno));
        goto done;
    }

so possibly the name of that structure member won't be a problem in sched.

@garlick
Copy link
Member

garlick commented Nov 8, 2017

This should be closed when #1277 is merged, since it makes the w member go away.

@chu11
Copy link
Member Author

chu11 commented Nov 8, 2017

it makes the w member go away in the handler spec, but I think it's still used as w in a lot of places (function parameters, etc.).

@garlick
Copy link
Member

garlick commented Nov 8, 2017

Oh OK, sounds good.

garlick added a commit to garlick/flux-core that referenced this issue Nov 9, 2017
Use a more sensible variable name for flux_msg_handler_t
than "w", etc..

Fixes flux-framework#1219
@chu11 chu11 closed this as completed in 952da93 Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants