[PATCH phosphor-host-ipmid v2] Add get/set ipmid command support with correct DBUS property handling.
Michael Neuling
mikey at neuling.org
Fri Dec 18 15:12:21 AEDT 2015
> +ipmi_ret_t ipmi_chassis_set_sys_boot_options(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
> + ipmi_request_t request, ipmi_response_t response,
> + ipmi_data_len_t data_len, ipmi_context_t context)
> +{
> + ipmi_ret_t rc = IPMI_CC_OK;
> +
> + printf("IPMI SET_SYS_BOOT_OPTIONS\n");
> + printf("IPMID set command required return bytes: %i\n", *data_len);
> +
> + set_sys_boot_options_t *reqptr = (set_sys_boot_options_t*) request;
> +
> + char* output_buf = (char*)malloc(NUM_RETURN_BYTES_OF_SET);
This seems to be allocating 1 byte. Why not just allocate this
locally on the stack with:
char output_buf;
then you can avoid the free and memory checks and a bunch of
other complexity.
I don't think you even need this buffer.
> +
> + if (output_buf == NULL) {
> + fprintf(stderr, "Malloc failed for set_sys_boot_options.\n");
> + return IPMI_OUT_OF_SPACE;
> + }
> +
> + if (reqptr->parameter == 5) // Parameter #5
> + {
> + char input_buf[NUM_INPUT_BYTES_OF_SET*2 + 1];
> + sprintf(input_buf, "%llX", reqptr->data.data64);
> +
> + int r = dbus_set_property(input_buf);
> +
> + if (r < 0) {
> + fprintf(stderr, "Dbus set property failed for set_sys_boot_options.\n");
> + return IPMI_OUT_OF_SPACE;
> + }
> +
> + *data_len = NUM_RETURN_BYTES_OF_SET;
> + // 0x0: return code OK.
> + output_buf[0] = 0x0;
> + memcpy(response, output_buf, *data_len);
This is a memcpy of 1 byte. Just assign it and remove the rest of the code.
*response = 0;
> + free(output_buf);
> + }
> + else
> + {
> + // 0x80: parameter not supported
> + output_buf[0] = 0x80;
> + memcpy(response, output_buf, *data_len);
*data_len is passed in, but output_buf is only 1 byte? Doesn't make
overflowing output_buf really easy?
Or should this be a 1 byte copy too and then we can just assign it
with something like:
*response = 0x80;
Mikey
More information about the openbmc
mailing list