Changeset View
Standalone View
sys/net/if_var.h
Context not available. | |||||
/* Interface link state change event */ | /* Interface link state change event */ | ||||
typedef void (*ifnet_link_event_handler_t)(void *, struct ifnet *, int); | typedef void (*ifnet_link_event_handler_t)(void *, struct ifnet *, int); | ||||
EVENTHANDLER_DECLARE(ifnet_link_event, ifnet_link_event_handler_t); | EVENTHANDLER_DECLARE(ifnet_link_event, ifnet_link_event_handler_t); | ||||
/* Interface up/down event */ | |||||
dab: I would stick with the previous "ifup/ifdown" or go with "Interface up/down". I think… | |||||
Not Done Inline ActionsI was trying to keep the consistency with Line 392, 395, 401 and 404. decui_microsoft.com: I was trying to keep the consistency with Line 392, 395, 401 and 404.
I would tend to leave the… | |||||
Not Done Inline ActionsNo objection to "Interface ..." but then I'd use "Interface up/down event" (the "if" in "ifdown" meaning "interface" is redundant). But, then, this is a nit. I don't strongly object to the patch as-is. dab: No objection to "Interface ..." but then I'd use "Interface up/down event" (the "if" in… | |||||
Not Done Inline ActionsOh... Sorry, my bad! I failed to understand what you meant -- it's late at night here and obviously my eyes or brain didn't work very good :-) decui_microsoft.com: Oh... Sorry, my bad! I failed to understand what you meant -- it's late at night here and… | |||||
#define IFNET_EVENT_UP 0 | |||||
Not Done Inline ActionsIt would be nice to have the 0/1 values for these two defines line up. In any case, it definitely seems like there is extraneous whitespace between "IFNET_EVENT_UP" and "0". dab: It would be nice to have the 0/1 values for these two defines line up. In any case, it… | |||||
Not Done Inline ActionsActually the 2 lines do line up. With "set list" in my Vim, the lines of the file net/if_var.h (rather than the patch with a leading + at the beginning of the lines) shows #define IFNET_EVENT_UP^I^I0$ ^I means a Tab. decui_microsoft.com: Actually the 2 lines do line up.
The visual confusion is due to the nasty Tab issue in the case… | |||||
Not Done Inline ActionsHmmm, you are right; Phabricator is deceiving me. They don't line up at all in the diff display it shows me, but you are right that they do in code. Sorry about that. (Just a note that style(9) says to use just a single tab between name and value, which would mean they wouldn't line up. See similar situation at lines 185-186.) dab: Hmmm, you are right; Phabricator is deceiving me. They don't line up at all in the diff display… | |||||
#define IFNET_EVENT_DOWN 1 | |||||
typedef void (*ifnet_event_fn)(void *, struct ifnet *ifp, int event); | |||||
EVENTHANDLER_DECLARE(ifnet_event, ifnet_event_fn); | |||||
#endif /* _SYS_EVENTHANDLER_H_ */ | #endif /* _SYS_EVENTHANDLER_H_ */ | ||||
/* | /* | ||||
Context not available. |
I would stick with the previous "ifup/ifdown" or go with "Interface up/down". I think "Interface up/ifdown" reads poorly.