diff mbox

[v3,02/10] ipmi: replace goto by a return statement

Message ID 1453730855-29302-3-git-send-email-clg@fr.ibm.com
State New
Headers show

Commit Message

Cédric Le Goater Jan. 25, 2016, 2:07 p.m. UTC 
Each routine using the IPMI_ADD_RSP_DATA, IPMI_CHECK_CMD_LEN or
IPMI_CHECK_RESERVATION macros needs to define a goto label 'out' to
handle hidden errors. Using directly a return statement has the same
effect and it removes the fact that 'out' needs to be defined.

The code exits in ipmi_sim_handle_command() are a little different
from the rest and a "possible" error in the macro IPMI_ADD_RSP_DATA is
handled before making use of it. This might be a bit excessive as a
minimum response len is currently 300 bytes and the patch checks that
at least 3 are available.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 140 +++++++++++++++----------------------------------
 1 file changed, 41 insertions(+), 99 deletions(-)
diff mbox

Patch

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 0a59e539f549..e42c7e86c344 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -258,7 +258,7 @@  struct IPMIBmcSim {
     do {                                                   \
         if (*rsp_len >= max_rsp_len) {                     \
             rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;       \
-            goto out;                                      \
+            return;                                        \
         }                                                  \
         rsp[(*rsp_len)++] = (b);                           \
     } while (0)
@@ -267,7 +267,7 @@  struct IPMIBmcSim {
 #define IPMI_CHECK_CMD_LEN(l) \
     if (cmd_len < l) {                                     \
         rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;      \
-        goto out; \
+        return; \
     }
 
 /* Check that the reservation in the command is valid. */
@@ -275,7 +275,7 @@  struct IPMIBmcSim {
     do {                                                   \
         if ((cmd[off] | (cmd[off + 1] << 8)) != r) {       \
             rsp[2] = IPMI_CC_INVALID_RESERVATION;          \
-            goto out;                                      \
+            return;                                        \
         }                                                  \
     } while (0)
 
@@ -453,14 +453,12 @@  static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
     }
 
     if (ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL) {
-        goto out;
+        return;
     }
 
     memcpy(ibs->evtbuf, evt, 16);
     ibs->msg_flags |= IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
     k->set_atn(s, 1, attn_irq_enabled(ibs));
- out:
-    return;
 }
 
 static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
@@ -581,6 +579,11 @@  static void ipmi_sim_handle_command(IPMIBmc *b,
 
     /* Set up the response, set the low bit of NETFN. */
     /* Note that max_rsp_len must be at least 3 */
+    if (max_rsp_len < 3) {
+        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+        goto out;
+    }
+
     IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
     IPMI_ADD_RSP_DATA(cmd[1]);
     IPMI_ADD_RSP_DATA(0); /* Assume success */
@@ -698,8 +701,6 @@  static void chassis_capabilities(IPMIBmcSim *ibs,
     IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
     IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
     IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
- out:
-    return;
 }
 
 static void chassis_status(IPMIBmcSim *ibs,
@@ -711,8 +712,6 @@  static void chassis_status(IPMIBmcSim *ibs,
     IPMI_ADD_RSP_DATA(0);
     IPMI_ADD_RSP_DATA(0);
     IPMI_ADD_RSP_DATA(0);
- out:
-    return;
 }
 
 static void chassis_control(IPMIBmcSim *ibs,
@@ -746,10 +745,8 @@  static void chassis_control(IPMIBmcSim *ibs,
         break;
     default:
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
- out:
-    return;
 }
 
 static void get_device_id(IPMIBmcSim *ibs,
@@ -768,8 +765,6 @@  static void get_device_id(IPMIBmcSim *ibs,
     IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
     IPMI_ADD_RSP_DATA(ibs->product_id[0]);
     IPMI_ADD_RSP_DATA(ibs->product_id[1]);
- out:
-    return;
 }
 
 static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
@@ -822,8 +817,6 @@  static void set_bmc_global_enables(IPMIBmcSim *ibs,
 {
     IPMI_CHECK_CMD_LEN(3);
     set_global_enables(ibs, cmd[2]);
- out:
-    return;
 }
 
 static void get_bmc_global_enables(IPMIBmcSim *ibs,
@@ -832,8 +825,6 @@  static void get_bmc_global_enables(IPMIBmcSim *ibs,
                                    unsigned int max_rsp_len)
 {
     IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
- out:
-    return;
 }
 
 static void clr_msg_flags(IPMIBmcSim *ibs,
@@ -847,8 +838,6 @@  static void clr_msg_flags(IPMIBmcSim *ibs,
     IPMI_CHECK_CMD_LEN(3);
     ibs->msg_flags &= ~cmd[2];
     k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
- out:
-    return;
 }
 
 static void get_msg_flags(IPMIBmcSim *ibs,
@@ -857,8 +846,6 @@  static void get_msg_flags(IPMIBmcSim *ibs,
                           unsigned int max_rsp_len)
 {
     IPMI_ADD_RSP_DATA(ibs->msg_flags);
- out:
-    return;
 }
 
 static void read_evt_msg_buf(IPMIBmcSim *ibs,
@@ -872,15 +859,13 @@  static void read_evt_msg_buf(IPMIBmcSim *ibs,
 
     if (!(ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL)) {
         rsp[2] = 0x80;
-        goto out;
+        return;
     }
     for (i = 0; i < 16; i++) {
         IPMI_ADD_RSP_DATA(ibs->evtbuf[i]);
     }
     ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
     k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
- out:
-    return;
 }
 
 static void get_msg(IPMIBmcSim *ibs,
@@ -911,7 +896,7 @@  static void get_msg(IPMIBmcSim *ibs,
         k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
     }
 
- out:
+out:
     qemu_mutex_unlock(&ibs->lock);
     return;
 }
@@ -942,14 +927,14 @@  static void send_msg(IPMIBmcSim *ibs,
     if (cmd[2] != 0) {
         /* We only handle channel 0 with no options */
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
 
     IPMI_CHECK_CMD_LEN(10);
     if (cmd[3] != 0x40) {
         /* We only emulate a MC at address 0x40. */
         rsp[2] = 0x83; /* NAK on write */
-        goto out;
+        return;
     }
 
     cmd += 3; /* Skip the header. */
@@ -961,7 +946,7 @@  static void send_msg(IPMIBmcSim *ibs,
      */
     if (ipmb_checksum(cmd, cmd_len, 0) != 0 ||
         cmd[3] != 0x20) { /* Improper response address */
-        goto out; /* No response */
+        return; /* No response */
     }
 
     netfn = cmd[1] >> 2;
@@ -971,7 +956,7 @@  static void send_msg(IPMIBmcSim *ibs,
 
     if (rqLun != 2) {
         /* We only support LUN 2 coming back to us. */
-        goto out;
+        return;
     }
 
     msg = g_malloc(sizeof(*msg));
@@ -1011,9 +996,6 @@  static void send_msg(IPMIBmcSim *ibs,
     ibs->msg_flags |= IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE;
     k->set_atn(s, 1, attn_irq_enabled(ibs));
     qemu_mutex_unlock(&ibs->lock);
-
- out:
-    return;
 }
 
 static void do_watchdog_reset(IPMIBmcSim *ibs)
@@ -1042,11 +1024,9 @@  static void reset_watchdog_timer(IPMIBmcSim *ibs,
 {
     if (!ibs->watchdog_initialized) {
         rsp[2] = 0x80;
-        goto out;
+        return;
     }
     do_watchdog_reset(ibs);
- out:
-    return;
 }
 
 static void set_watchdog_timer(IPMIBmcSim *ibs,
@@ -1062,7 +1042,7 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
     val = cmd[2] & 0x7; /* Validate use */
     if (val == 0 || val > 5) {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
     val = cmd[3] & 0x7; /* Validate action */
     switch (val) {
@@ -1086,7 +1066,7 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
     }
     if (rsp[2]) {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
 
     val = (cmd[3] >> 4) & 0x7; /* Validate preaction */
@@ -1099,12 +1079,12 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
         if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
             /* NMI not supported. */
             rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-            goto out;
+            return;
         }
     default:
         /* We don't support PRE_SMI */
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
 
     ibs->watchdog_initialized = 1;
@@ -1118,8 +1098,6 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
     } else {
         ibs->watchdog_running = 0;
     }
- out:
-    return;
 }
 
 static void get_watchdog_timer(IPMIBmcSim *ibs,
@@ -1141,8 +1119,6 @@  static void get_watchdog_timer(IPMIBmcSim *ibs,
         IPMI_ADD_RSP_DATA(0);
         IPMI_ADD_RSP_DATA(0);
     }
- out:
-    return;
 }
 
 static void get_sdr_rep_info(IPMIBmcSim *ibs,
@@ -1165,8 +1141,6 @@  static void get_sdr_rep_info(IPMIBmcSim *ibs,
     }
     /* Only modal support, reserve supported */
     IPMI_ADD_RSP_DATA((ibs->sdr.overflow << 7) | 0x22);
- out:
-    return;
 }
 
 static void reserve_sdr_rep(IPMIBmcSim *ibs,
@@ -1176,8 +1150,6 @@  static void reserve_sdr_rep(IPMIBmcSim *ibs,
 {
     IPMI_ADD_RSP_DATA(ibs->sdr.reservation & 0xff);
     IPMI_ADD_RSP_DATA((ibs->sdr.reservation >> 8) & 0xff);
- out:
-    return;
 }
 
 static void get_sdr(IPMIBmcSim *ibs,
@@ -1196,11 +1168,11 @@  static void get_sdr(IPMIBmcSim *ibs,
     if (sdr_find_entry(&ibs->sdr, cmd[4] | (cmd[5] << 8),
                        &pos, &nextrec)) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     if (cmd[6] > (ibs->sdr.sdr[pos + 4])) {
         rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
-        goto out;
+        return;
     }
 
     IPMI_ADD_RSP_DATA(nextrec & 0xff);
@@ -1212,12 +1184,10 @@  static void get_sdr(IPMIBmcSim *ibs,
 
     if ((cmd[7] + *rsp_len) > max_rsp_len) {
         rsp[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
-        goto out;
+        return;
     }
     memcpy(rsp + *rsp_len, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
     *rsp_len += cmd[7];
- out:
-    return;
 }
 
 static void add_sdr(IPMIBmcSim *ibs,
@@ -1229,12 +1199,10 @@  static void add_sdr(IPMIBmcSim *ibs,
 
     if (sdr_add_entry(ibs, cmd + 2, cmd_len - 2, &recid)) {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
     IPMI_ADD_RSP_DATA(recid & 0xff);
     IPMI_ADD_RSP_DATA((recid >> 8) & 0xff);
- out:
-    return;
 }
 
 static void clear_sdr_rep(IPMIBmcSim *ibs,
@@ -1246,7 +1214,7 @@  static void clear_sdr_rep(IPMIBmcSim *ibs,
     IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation);
     if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
     if (cmd[7] == 0xaa) {
         ibs->sdr.next_free = 0;
@@ -1258,10 +1226,8 @@  static void clear_sdr_rep(IPMIBmcSim *ibs,
         IPMI_ADD_RSP_DATA(1); /* Erasure complete */
     } else {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
- out:
-    return;
 }
 
 static void get_sel_info(IPMIBmcSim *ibs,
@@ -1285,8 +1251,6 @@  static void get_sel_info(IPMIBmcSim *ibs,
     }
     /* Only support Reserve SEL */
     IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02);
- out:
-    return;
 }
 
 static void reserve_sel(IPMIBmcSim *ibs,
@@ -1296,8 +1260,6 @@  static void reserve_sel(IPMIBmcSim *ibs,
 {
     IPMI_ADD_RSP_DATA(ibs->sel.reservation & 0xff);
     IPMI_ADD_RSP_DATA((ibs->sel.reservation >> 8) & 0xff);
- out:
-    return;
 }
 
 static void get_sel_entry(IPMIBmcSim *ibs,
@@ -1313,17 +1275,17 @@  static void get_sel_entry(IPMIBmcSim *ibs,
     }
     if (ibs->sel.next_free == 0) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     if (cmd[6] > 15) {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
     if (cmd[7] == 0xff) {
         cmd[7] = 16;
     } else if ((cmd[7] + cmd[6]) > 16) {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     } else {
         cmd[7] += cmd[6];
     }
@@ -1333,7 +1295,7 @@  static void get_sel_entry(IPMIBmcSim *ibs,
         val = ibs->sel.next_free - 1;
     } else if (val >= ibs->sel.next_free) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     if ((val + 1) == ibs->sel.next_free) {
         IPMI_ADD_RSP_DATA(0xff);
@@ -1345,8 +1307,6 @@  static void get_sel_entry(IPMIBmcSim *ibs,
     for (; cmd[6] < cmd[7]; cmd[6]++) {
         IPMI_ADD_RSP_DATA(ibs->sel.sel[val][cmd[6]]);
     }
- out:
-    return;
 }
 
 static void add_sel_entry(IPMIBmcSim *ibs,
@@ -1357,13 +1317,11 @@  static void add_sel_entry(IPMIBmcSim *ibs,
     IPMI_CHECK_CMD_LEN(18);
     if (sel_add_event(ibs, cmd + 2)) {
         rsp[2] = IPMI_CC_OUT_OF_SPACE;
-        goto out;
+        return;
     }
     /* sel_add_event fills in the record number. */
     IPMI_ADD_RSP_DATA(cmd[2]);
     IPMI_ADD_RSP_DATA(cmd[3]);
- out:
-    return;
 }
 
 static void clear_sel(IPMIBmcSim *ibs,
@@ -1375,7 +1333,7 @@  static void clear_sel(IPMIBmcSim *ibs,
     IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
     if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
     if (cmd[7] == 0xaa) {
         ibs->sel.next_free = 0;
@@ -1387,10 +1345,8 @@  static void clear_sel(IPMIBmcSim *ibs,
         IPMI_ADD_RSP_DATA(1); /* Erasure complete */
     } else {
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
- out:
-    return;
 }
 
 static void get_sel_time(IPMIBmcSim *ibs,
@@ -1407,8 +1363,6 @@  static void get_sel_time(IPMIBmcSim *ibs,
     IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
     IPMI_ADD_RSP_DATA((val >> 16) & 0xff);
     IPMI_ADD_RSP_DATA((val >> 24) & 0xff);
- out:
-    return;
 }
 
 static void set_sel_time(IPMIBmcSim *ibs,
@@ -1423,8 +1377,6 @@  static void set_sel_time(IPMIBmcSim *ibs,
     val = cmd[2] | (cmd[3] << 8) | (cmd[4] << 16) | (cmd[5] << 24);
     ipmi_gettime(&now);
     ibs->sel.time_offset = now.tv_sec - ((long) val);
- out:
-    return;
 }
 
 static void set_sensor_evt_enable(IPMIBmcSim *ibs,
@@ -1438,7 +1390,7 @@  static void set_sensor_evt_enable(IPMIBmcSim *ibs,
     if ((cmd[2] > MAX_SENSORS) ||
             !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     sens = ibs->sensors + cmd[2];
     switch ((cmd[3] >> 4) & 0x3) {
@@ -1474,11 +1426,9 @@  static void set_sensor_evt_enable(IPMIBmcSim *ibs,
         break;
     case 3:
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-        goto out;
+        return;
     }
     IPMI_SENSOR_SET_RET_STATUS(sens, cmd[3]);
- out:
-    return;
 }
 
 static void get_sensor_evt_enable(IPMIBmcSim *ibs,
@@ -1492,7 +1442,7 @@  static void get_sensor_evt_enable(IPMIBmcSim *ibs,
     if ((cmd[2] > MAX_SENSORS) ||
         !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     sens = ibs->sensors + cmd[2];
     IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
@@ -1500,8 +1450,6 @@  static void get_sensor_evt_enable(IPMIBmcSim *ibs,
     IPMI_ADD_RSP_DATA((sens->assert_enable >> 8) & 0xff);
     IPMI_ADD_RSP_DATA(sens->deassert_enable & 0xff);
     IPMI_ADD_RSP_DATA((sens->deassert_enable >> 8) & 0xff);
- out:
-    return;
 }
 
 static void rearm_sensor_evts(IPMIBmcSim *ibs,
@@ -1515,17 +1463,15 @@  static void rearm_sensor_evts(IPMIBmcSim *ibs,
     if ((cmd[2] > MAX_SENSORS) ||
         !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     sens = ibs->sensors + cmd[2];
 
     if ((cmd[3] & 0x80) == 0) {
         /* Just clear everything */
         sens->states = 0;
-        goto out;
+        return;
     }
- out:
-    return;
 }
 
 static void get_sensor_evt_status(IPMIBmcSim *ibs,
@@ -1539,7 +1485,7 @@  static void get_sensor_evt_status(IPMIBmcSim *ibs,
     if ((cmd[2] > MAX_SENSORS) ||
         !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     sens = ibs->sensors + cmd[2];
     IPMI_ADD_RSP_DATA(sens->reading);
@@ -1548,8 +1494,6 @@  static void get_sensor_evt_status(IPMIBmcSim *ibs,
     IPMI_ADD_RSP_DATA((sens->assert_states >> 8) & 0xff);
     IPMI_ADD_RSP_DATA(sens->deassert_states & 0xff);
     IPMI_ADD_RSP_DATA((sens->deassert_states >> 8) & 0xff);
- out:
-    return;
 }
 
 static void get_sensor_reading(IPMIBmcSim *ibs,
@@ -1563,7 +1507,7 @@  static void get_sensor_reading(IPMIBmcSim *ibs,
     if ((cmd[2] > MAX_SENSORS) ||
             !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
         rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
-        goto out;
+        return;
     }
     sens = ibs->sensors + cmd[2];
     IPMI_ADD_RSP_DATA(sens->reading);
@@ -1572,8 +1516,6 @@  static void get_sensor_reading(IPMIBmcSim *ibs,
     if (IPMI_SENSOR_IS_DISCRETE(sens)) {
         IPMI_ADD_RSP_DATA((sens->states >> 8) & 0xff);
     }
- out:
-    return;
 }
 
 static const IPMICmdHandler chassis_cmds[IPMI_NETFN_CHASSIS_MAXCMD] = {