On 03/09/2016 05:07 AM, Corey Minyard wrote: > On 03/09/2016 12:06 AM, Cédric Le Goater wrote: >> On 03/07/2016 11:40 AM, Cédric Le Goater wrote: >>> On 03/05/2016 12:41 PM, Corey Minyard wrote: >>>> On 03/02/2016 04:14 AM, Cédric Le Goater wrote: >>>>> Most IPMI command handlers in the BMC simulator start with a call to >>>>> the macro IPMI_CHECK_CMD_LEN() which verifies that a minimal number of >>>>> arguments expected by the command are indeed available. To achieve >>>>> this task, the macro implicitly uses local variables which is >>>>> misleading in the code. >>>>> >>>>> This patch adds a 'cmd_len_min' attribute to the struct IPMICmdHandler >>>>> defining the minimal number of arguments expected by the command and >>>>> moves this check in the global command handler ipmi_sim_handle_command(). >>>> This is much better. One style comment inline... >>>> >>>> Acked-by: Corey Minyard <cminy...@mvista.com> >>>> >>>>> Signed-off-by: Cédric Le Goater <c...@fr.ibm.com> >>>>> --- >>>>> hw/ipmi/ipmi_bmc_sim.c | 137 >>>>> ++++++++++++++++++++++--------------------------- >>>>> 1 file changed, 62 insertions(+), 75 deletions(-) >>>>> >>>>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c >>>>> index 51d234aa1bf2..30b9fb48ea2d 100644 >>>>> --- a/hw/ipmi/ipmi_bmc_sim.c >>>>> +++ b/hw/ipmi/ipmi_bmc_sim.c >>>>> @@ -155,10 +155,15 @@ typedef struct IPMISensor { >>>>> typedef struct IPMIBmcSim IPMIBmcSim; >>>>> >>>>> #define MAX_NETFNS 64 >>>>> -typedef void (*IPMICmdHandler)(IPMIBmcSim *s, >>>>> - uint8_t *cmd, unsigned int cmd_len, >>>>> - uint8_t *rsp, unsigned int *rsp_len, >>>>> - unsigned int max_rsp_len); >>>>> + >>>>> +typedef struct IPMICmdHandler { >>>>> + void (*cmd_handler)(IPMIBmcSim *s, >>>>> + uint8_t *cmd, unsigned int cmd_len, >>>>> + uint8_t *rsp, unsigned int *rsp_len, >>>>> + unsigned int max_rsp_len); >>>>> + unsigned int cmd_len_min; >>>>> +} IPMICmdHandler; >>>>> + >>>>> typedef struct IPMINetfn { >>>>> unsigned int cmd_nums; >>>>> const IPMICmdHandler *cmd_handlers; >>>>> @@ -269,13 +274,6 @@ struct IPMIBmcSim { >>>>> rsp[(*rsp_len)++] = (b); \ >>>>> } while (0) >>>>> >>>>> -/* Verify that the received command is a certain length. */ >>>>> -#define IPMI_CHECK_CMD_LEN(l) \ >>>>> - if (cmd_len < l) { \ >>>>> - rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID; \ >>>>> - return; \ >>>>> - } >>>>> - >>>>> /* Check that the reservation in the command is valid. */ >>>>> #define IPMI_CHECK_RESERVATION(off, r) \ >>>>> do { \ >>>>> @@ -623,14 +621,19 @@ static void ipmi_sim_handle_command(IPMIBmc *b, >>>>> >>>>> /* Odd netfns are not valid, make sure the command is registered */ >>>>> if ((netfn & 1) || !ibs->netfns[netfn / 2] || >>>>> - (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) || >>>>> - (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]])) >>>>> { >>>>> + (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) || >>>>> + (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler)) { >>>>> rsp[2] = IPMI_CC_INVALID_CMD; >>>>> goto out; >>>> I'm not sure of the qemu style here, but the above change makes the code >>>> hard to read. >>> Cleary. I was lazy. I will try to clarify what this code does with a helper >>> or some intermediate variables. >> How's that ? >> >> @@ -591,6 +589,7 @@ static void ipmi_sim_handle_command(IPMI >> unsigned int rsp_len_holder = 0; >> unsigned int *rsp_len = &rsp_len_holder; >> unsigned int max_rsp_len = sizeof(rsp); >> + const IPMICmdHandler *hdl; >> /* Set up the response, set the low bit of NETFN. */ >> /* Note that max_rsp_len must be at least 3 */ >> @@ -623,14 +622,23 @@ static void ipmi_sim_handle_command(IPMI >> /* Odd netfns are not valid, make sure the command is registered */ >> if ((netfn & 1) || !ibs->netfns[netfn / 2] || >> - (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) || >> - (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]])) { >> + (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums)) { >> rsp[2] = IPMI_CC_INVALID_CMD; >> goto out; >> } >> > > This cleans things up a bit. The curse of a 4 character indent is that > the line that follows a multi-line if statement is at the same indent as > the lines in the if statement, so it's hard to tell where the if statement > ends and where the code in the if statement begins. This is not bad, > but if you have if statements like the gcc compiler (at least how the > gcc compiler used to be, haven't looked at it recently) it makes your > head hurt. > > The Linux indent rule for this is that you indent the second and later > lines of an if statement way over, or you break it up.
yeah and in that case, an extra routine would be better. I'll do what you first proposed. A check on netfn is also missing. C. > I looked through a few of the major qemu files, and I couldn't find > a multi-line if statement, so it's hard to say. That is perhaps telling :). > > -corey > >> - ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]](ibs, cmd, cmd_len, rsp, >> rsp_len, >> - max_rsp_len); >> + hdl = &ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]]; >> + if (!hdl->cmd_handler) { >> + rsp[2] = IPMI_CC_INVALID_CMD; >> + goto out; >> + } >> + >> + if (cmd_len < hdl->cmd_len_min) { >> + rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID; >> + goto out; >> + } >> + >> + hdl->cmd_handler(ibs, cmd, cmd_len, rsp, rsp_len, max_rsp_len); >> out: >> k->handle_rsp(s, msg_id, rsp, *rsp_len); >> >> >> >> The function still fits in one emacs page, so I think we are fine. >> >> Just waiting a bit for Marcel to chime in before sending a v3. >> >> Thanks, >> C. >> >> >>>> The qemu style really seems to be to not have big if statements like >>>> this, so maybe this crasy check should be moved to a separate function? >>>> >>>>> } >>>>> >>>>> - ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]](ibs, cmd, cmd_len, rsp, >>>>> rsp_len, >>>>> - max_rsp_len); >>>>> + if (cmd_len < ibs->netfns[netfn / >>>>> 2]->cmd_handlers[cmd[1]].cmd_len_min) { >>>>> + rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID; >>>>> + goto out; >>>>> + } >>>>> + >>>>> + ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler(ibs, cmd, >>>>> cmd_len, >>>>> + rsp, rsp_len, >>>>> max_rsp_len); >>>>> >>>>> out: >>>>> k->handle_rsp(s, msg_id, rsp, *rsp_len); >>>>> @@ -737,7 +740,6 @@ static void chassis_control(IPMIBmcSim *ibs, >>>>> IPMIInterface *s = ibs->parent.intf; >>>>> IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s); >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> switch (cmd[2] & 0xf) { >>>>> case 0: /* power down */ >>>>> rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0); >>>>> @@ -838,7 +840,6 @@ static void set_acpi_power_state(IPMIBmcSim *ibs, >>>>> uint8_t *rsp, unsigned int *rsp_len, >>>>> unsigned int max_rsp_len) >>>>> { >>>>> - IPMI_CHECK_CMD_LEN(4); >>>>> ibs->acpi_power_state[0] = cmd[2]; >>>>> ibs->acpi_power_state[1] = cmd[3]; >>>>> } >>>>> @@ -869,7 +870,6 @@ static void set_bmc_global_enables(IPMIBmcSim *ibs, >>>>> uint8_t *rsp, unsigned int *rsp_len, >>>>> unsigned int max_rsp_len) >>>>> { >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> set_global_enables(ibs, cmd[2]); >>>>> } >>>>> >>>>> @@ -889,7 +889,6 @@ static void clr_msg_flags(IPMIBmcSim *ibs, >>>>> IPMIInterface *s = ibs->parent.intf; >>>>> IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s); >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> ibs->msg_flags &= ~cmd[2]; >>>>> k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs)); >>>>> } >>>>> @@ -976,15 +975,17 @@ static void send_msg(IPMIBmcSim *ibs, >>>>> uint8_t *buf; >>>>> uint8_t netfn, rqLun, rsLun, rqSeq; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> - >>>>> if (cmd[2] != 0) { >>>>> /* We only handle channel 0 with no options */ >>>>> rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> return; >>>>> } >>>>> >>>>> - IPMI_CHECK_CMD_LEN(10); >>>>> + if (cmd_len < 10) { >>>>> + rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID; >>>>> + return; >>>>> + } >>>>> + >>>>> if (cmd[3] != 0x40) { >>>>> /* We only emulate a MC at address 0x40. */ >>>>> rsp[2] = 0x83; /* NAK on write */ >>>>> @@ -1092,7 +1093,6 @@ static void set_watchdog_timer(IPMIBmcSim *ibs, >>>>> IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s); >>>>> unsigned int val; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(8); >>>>> val = cmd[2] & 0x7; /* Validate use */ >>>>> if (val == 0 || val > 5) { >>>>> rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> @@ -1217,7 +1217,6 @@ static void get_sdr(IPMIBmcSim *ibs, >>>>> uint16_t nextrec; >>>>> struct ipmi_sdr_header *sdrh; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(8); >>>>> if (cmd[6]) { >>>>> IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation); >>>>> } >>>>> @@ -1271,7 +1270,6 @@ static void clear_sdr_rep(IPMIBmcSim *ibs, >>>>> uint8_t *rsp, unsigned int *rsp_len, >>>>> unsigned int max_rsp_len) >>>>> { >>>>> - IPMI_CHECK_CMD_LEN(8); >>>>> IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation); >>>>> if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') { >>>>> rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> @@ -1330,7 +1328,6 @@ static void get_sel_entry(IPMIBmcSim *ibs, >>>>> { >>>>> unsigned int val; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(8); >>>>> if (cmd[6]) { >>>>> IPMI_CHECK_RESERVATION(2, ibs->sel.reservation); >>>>> } >>>>> @@ -1375,7 +1372,6 @@ static void add_sel_entry(IPMIBmcSim *ibs, >>>>> uint8_t *rsp, unsigned int *rsp_len, >>>>> unsigned int max_rsp_len) >>>>> { >>>>> - IPMI_CHECK_CMD_LEN(18); >>>>> if (sel_add_event(ibs, cmd + 2)) { >>>>> rsp[2] = IPMI_CC_OUT_OF_SPACE; >>>>> return; >>>>> @@ -1390,7 +1386,6 @@ static void clear_sel(IPMIBmcSim *ibs, >>>>> uint8_t *rsp, unsigned int *rsp_len, >>>>> unsigned int max_rsp_len) >>>>> { >>>>> - IPMI_CHECK_CMD_LEN(8); >>>>> IPMI_CHECK_RESERVATION(2, ibs->sel.reservation); >>>>> if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') { >>>>> rsp[2] = IPMI_CC_INVALID_DATA_FIELD; >>>>> @@ -1434,7 +1429,6 @@ static void set_sel_time(IPMIBmcSim *ibs, >>>>> uint32_t val; >>>>> struct ipmi_time now; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(6); >>>>> val = cmd[2] | (cmd[3] << 8) | (cmd[4] << 16) | (cmd[5] << 24); >>>>> ipmi_gettime(&now); >>>>> ibs->sel.time_offset = now.tv_sec - ((long) val); >>>>> @@ -1447,7 +1441,6 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs, >>>>> { >>>>> IPMISensor *sens; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(4); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1499,7 +1492,6 @@ static void get_sensor_evt_enable(IPMIBmcSim *ibs, >>>>> { >>>>> IPMISensor *sens; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1520,7 +1512,6 @@ static void rearm_sensor_evts(IPMIBmcSim *ibs, >>>>> { >>>>> IPMISensor *sens; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(4); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1542,7 +1533,6 @@ static void get_sensor_evt_status(IPMIBmcSim *ibs, >>>>> { >>>>> IPMISensor *sens; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1564,7 +1554,6 @@ static void get_sensor_reading(IPMIBmcSim *ibs, >>>>> { >>>>> IPMISensor *sens; >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1587,7 +1576,6 @@ static void set_sensor_type(IPMIBmcSim *ibs, >>>>> IPMISensor *sens; >>>>> >>>>> >>>>> - IPMI_CHECK_CMD_LEN(5); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1606,7 +1594,6 @@ static void get_sensor_type(IPMIBmcSim *ibs, >>>>> IPMISensor *sens; >>>>> >>>>> >>>>> - IPMI_CHECK_CMD_LEN(3); >>>>> if ((cmd[2] >= MAX_SENSORS) || >>>>> !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { >>>>> rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT; >>>>> @@ -1619,10 +1606,10 @@ static void get_sensor_type(IPMIBmcSim *ibs, >>>>> >>>>> >>>>> static const IPMICmdHandler chassis_cmds[] = { >>>>> - [IPMI_CMD_GET_CHASSIS_CAPABILITIES] = chassis_capabilities, >>>>> - [IPMI_CMD_GET_CHASSIS_STATUS] = chassis_status, >>>>> - [IPMI_CMD_CHASSIS_CONTROL] = chassis_control, >>>>> - [IPMI_CMD_GET_SYS_RESTART_CAUSE] = chassis_get_sys_restart_cause >>>>> + [IPMI_CMD_GET_CHASSIS_CAPABILITIES] = { chassis_capabilities }, >>>>> + [IPMI_CMD_GET_CHASSIS_STATUS] = { chassis_status }, >>>>> + [IPMI_CMD_CHASSIS_CONTROL] = { chassis_control, 3 }, >>>>> + [IPMI_CMD_GET_SYS_RESTART_CAUSE] = { chassis_get_sys_restart_cause } >>>>> }; >>>>> static const IPMINetfn chassis_netfn = { >>>>> .cmd_nums = ARRAY_SIZE(chassis_cmds), >>>>> @@ -1630,13 +1617,13 @@ static const IPMINetfn chassis_netfn = { >>>>> }; >>>>> >>>>> static const IPMICmdHandler sensor_event_cmds[] = { >>>>> - [IPMI_CMD_SET_SENSOR_EVT_ENABLE] = set_sensor_evt_enable, >>>>> - [IPMI_CMD_GET_SENSOR_EVT_ENABLE] = get_sensor_evt_enable, >>>>> - [IPMI_CMD_REARM_SENSOR_EVTS] = rearm_sensor_evts, >>>>> - [IPMI_CMD_GET_SENSOR_EVT_STATUS] = get_sensor_evt_status, >>>>> - [IPMI_CMD_GET_SENSOR_READING] = get_sensor_reading, >>>>> - [IPMI_CMD_SET_SENSOR_TYPE] = set_sensor_type, >>>>> - [IPMI_CMD_GET_SENSOR_TYPE] = get_sensor_type, >>>>> + [IPMI_CMD_SET_SENSOR_EVT_ENABLE] = { set_sensor_evt_enable, 4 }, >>>>> + [IPMI_CMD_GET_SENSOR_EVT_ENABLE] = { get_sensor_evt_enable, 3 }, >>>>> + [IPMI_CMD_REARM_SENSOR_EVTS] = { rearm_sensor_evts, 4 }, >>>>> + [IPMI_CMD_GET_SENSOR_EVT_STATUS] = { get_sensor_evt_status, 3 }, >>>>> + [IPMI_CMD_GET_SENSOR_READING] = { get_sensor_reading, 3 }, >>>>> + [IPMI_CMD_SET_SENSOR_TYPE] = { set_sensor_type, 5 }, >>>>> + [IPMI_CMD_GET_SENSOR_TYPE] = { get_sensor_type, 3 }, >>>>> }; >>>>> static const IPMINetfn sensor_event_netfn = { >>>>> .cmd_nums = ARRAY_SIZE(sensor_event_cmds), >>>>> @@ -1644,22 +1631,22 @@ static const IPMINetfn sensor_event_netfn = { >>>>> }; >>>>> >>>>> static const IPMICmdHandler app_cmds[] = { >>>>> - [IPMI_CMD_GET_DEVICE_ID] = get_device_id, >>>>> - [IPMI_CMD_COLD_RESET] = cold_reset, >>>>> - [IPMI_CMD_WARM_RESET] = warm_reset, >>>>> - [IPMI_CMD_SET_ACPI_POWER_STATE] = set_acpi_power_state, >>>>> - [IPMI_CMD_GET_ACPI_POWER_STATE] = get_acpi_power_state, >>>>> - [IPMI_CMD_GET_DEVICE_GUID] = get_device_guid, >>>>> - [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = set_bmc_global_enables, >>>>> - [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = get_bmc_global_enables, >>>>> - [IPMI_CMD_CLR_MSG_FLAGS] = clr_msg_flags, >>>>> - [IPMI_CMD_GET_MSG_FLAGS] = get_msg_flags, >>>>> - [IPMI_CMD_GET_MSG] = get_msg, >>>>> - [IPMI_CMD_SEND_MSG] = send_msg, >>>>> - [IPMI_CMD_READ_EVT_MSG_BUF] = read_evt_msg_buf, >>>>> - [IPMI_CMD_RESET_WATCHDOG_TIMER] = reset_watchdog_timer, >>>>> - [IPMI_CMD_SET_WATCHDOG_TIMER] = set_watchdog_timer, >>>>> - [IPMI_CMD_GET_WATCHDOG_TIMER] = get_watchdog_timer, >>>>> + [IPMI_CMD_GET_DEVICE_ID] = { get_device_id }, >>>>> + [IPMI_CMD_COLD_RESET] = { cold_reset }, >>>>> + [IPMI_CMD_WARM_RESET] = { warm_reset }, >>>>> + [IPMI_CMD_SET_ACPI_POWER_STATE] = { set_acpi_power_state, 4 }, >>>>> + [IPMI_CMD_GET_ACPI_POWER_STATE] = { get_acpi_power_state }, >>>>> + [IPMI_CMD_GET_DEVICE_GUID] = { get_device_guid }, >>>>> + [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = { set_bmc_global_enables, 3 }, >>>>> + [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = { get_bmc_global_enables }, >>>>> + [IPMI_CMD_CLR_MSG_FLAGS] = { clr_msg_flags, 3 }, >>>>> + [IPMI_CMD_GET_MSG_FLAGS] = { get_msg_flags }, >>>>> + [IPMI_CMD_GET_MSG] = { get_msg }, >>>>> + [IPMI_CMD_SEND_MSG] = { send_msg, 3 }, >>>>> + [IPMI_CMD_READ_EVT_MSG_BUF] = { read_evt_msg_buf }, >>>>> + [IPMI_CMD_RESET_WATCHDOG_TIMER] = { reset_watchdog_timer }, >>>>> + [IPMI_CMD_SET_WATCHDOG_TIMER] = { set_watchdog_timer, 8 }, >>>>> + [IPMI_CMD_GET_WATCHDOG_TIMER] = { get_watchdog_timer }, >>>>> }; >>>>> static const IPMINetfn app_netfn = { >>>>> .cmd_nums = ARRAY_SIZE(app_cmds), >>>>> @@ -1667,18 +1654,18 @@ static const IPMINetfn app_netfn = { >>>>> }; >>>>> >>>>> static const IPMICmdHandler storage_cmds[] = { >>>>> - [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info, >>>>> - [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep, >>>>> - [IPMI_CMD_GET_SDR] = get_sdr, >>>>> - [IPMI_CMD_ADD_SDR] = add_sdr, >>>>> - [IPMI_CMD_CLEAR_SDR_REP] = clear_sdr_rep, >>>>> - [IPMI_CMD_GET_SEL_INFO] = get_sel_info, >>>>> - [IPMI_CMD_RESERVE_SEL] = reserve_sel, >>>>> - [IPMI_CMD_GET_SEL_ENTRY] = get_sel_entry, >>>>> - [IPMI_CMD_ADD_SEL_ENTRY] = add_sel_entry, >>>>> - [IPMI_CMD_CLEAR_SEL] = clear_sel, >>>>> - [IPMI_CMD_GET_SEL_TIME] = get_sel_time, >>>>> - [IPMI_CMD_SET_SEL_TIME] = set_sel_time, >>>>> + [IPMI_CMD_GET_SDR_REP_INFO] = { get_sdr_rep_info }, >>>>> + [IPMI_CMD_RESERVE_SDR_REP] = { reserve_sdr_rep }, >>>>> + [IPMI_CMD_GET_SDR] = { get_sdr, 8 }, >>>>> + [IPMI_CMD_ADD_SDR] = { add_sdr }, >>>>> + [IPMI_CMD_CLEAR_SDR_REP] = { clear_sdr_rep, 8 }, >>>>> + [IPMI_CMD_GET_SEL_INFO] = { get_sel_info }, >>>>> + [IPMI_CMD_RESERVE_SEL] = { reserve_sel }, >>>>> + [IPMI_CMD_GET_SEL_ENTRY] = { get_sel_entry, 8 }, >>>>> + [IPMI_CMD_ADD_SEL_ENTRY] = { add_sel_entry, 18 }, >>>>> + [IPMI_CMD_CLEAR_SEL] = { clear_sel, 8 }, >>>>> + [IPMI_CMD_GET_SEL_TIME] = { get_sel_time, 6 }, >>>>> + [IPMI_CMD_SET_SEL_TIME] = { set_sel_time }, >>>>> }; >>>>> >>>>> static const IPMINetfn storage_netfn = { >