Re: [PATCH phosphor-host-ipmid v2] Add get/set ipmid command support with correct DBUS property handling.

[Stewart Smith <stewart@linux.vnet.ibm.com>]

OpenBMC Patches <openbmc-patches@stwcx.xyz> writes:
> +struct set_sys_boot_options_t {
> +    uint8_t parameter;
> +    union {
> +        struct {
> +            uint8_t d1;
> +            uint8_t d2;
> +            uint8_t d3;
> +            uint8_t d4;
> +            uint8_t d5;
> +            uint8_t d6;
> +            uint8_t d7;
> +            uint8_t d8;
> +        } data8;
> +        uint64_t data64;
> +    } data;
> +}  __attribute__ ((packed));

How is this going to be used in an endian safe manner?

Why not use sparse annotations?

> +
>  ipmi_ret_t ipmi_chassis_wildcard(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)
> @@ -116,16 +332,41 @@ ipmi_ret_t ipmi_chassis_get_sys_boot_options(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
>  
>      get_sys_boot_options_t *reqptr = (get_sys_boot_options_t*) request;
>  
> -    // TODO Return default values to OPAL until dbus interface is available
> +    char* buf = (char*)malloc(NUM_RETURN_BYTES_OF_GET*2 + 1);
> +
> +    if (buf == NULL) {
> +        fprintf(stderr, "Malloc failed for get_sys_boot_options.\n");
> +        return IPMI_OUT_OF_SPACE;        

trailing whitespace

>      if (reqptr->parameter == 5) // Parameter #5

Why 5 ?

comment is redundantly redundant.

>      {
> -        uint8_t buf[] = {0x1,0x5,80,0,0,0,0};
> -        *data_len = sizeof(buf);
> -        memcpy(response, &buf, *data_len);
> +        int r = dbus_get_property(buf);
> +
> +        if (r < 0) {
> +            fprintf(stderr, "Dbus get property failed for get_sys_boot_options.\n");
> +            return IPMI_OUT_OF_SPACE;        
> +        }

trailing whitespace