[PATCH phosphor-host-ipmid] Add Set MAC address

[Joel Stanley]

On Thu, Feb 4, 2016 at 10:40 AM, OpenBMC Patches
<openbmc-patches at stwcx.xyz> wrote:
> From: Adriana Kobylak <anoo at us.ibm.com>
>
> Add Set MAC address parameter command

I notice you posted this patch 40 minutes ago, but had already merged
it. That doesn't give much opportunity for review.

> ---
>  transporthandler.C | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/transporthandler.C b/transporthandler.C
> index dac8fe8..c38adea 100644
> --- a/transporthandler.C
> +++ b/transporthandler.C
> @@ -14,6 +14,7 @@
>  #endif
>
>  // OpenBMC System Manager dbus framework
> +extern sd_bus *bus;

Don't use extern.

We have ipmid_get_sd_bus_connection() to get a pointer to the sd_bus.

>  const char  *app   =  "org.openbmc.NetworkManager";
>  const char  *obj   =  "/org/openbmc/NetworkManager/Interface";
>  const char  *ifc   =  "org.openbmc.NetworkManager";
> @@ -64,6 +65,37 @@ ipmi_ret_t ipmi_transport_set_lan(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
>      {
>          sprintf(new_ipaddr, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
>      }
> +    else if (reqptr->parameter == 5) // MAC
> +    {
> +        char                mac[18];
> +        sd_bus_message *reply = NULL, *m = NULL;
> +        sd_bus_error error = SD_BUS_ERROR_NULL;
> +        int r = 0;

I don't think any of these should be initialised here.

> +
> +        sprintf(mac, "%x:%x:%x:%x:%x:%x",

You want to use snprintf here.

Also, does this work? I would have thought you'd need to do %02x to
ensure leading zeroes are in the string.

> +                reqptr->data[0],
> +                reqptr->data[1],
> +                reqptr->data[2],
> +                reqptr->data[3],
> +                reqptr->data[4],
> +                reqptr->data[5]);
> +
> +        r = sd_bus_message_new_method_call(bus,&m,app,obj,ifc,"SetHwAddress");
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to add method object: %s\n", strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_message_append(m, "s", mac);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to append message data: %s\n", strerror(-r));
> +            return -1;
> +        }
> +        r = sd_bus_call(bus, m, 0, &error, &reply);
> +        if (r < 0) {
> +            fprintf(stderr, "Failed to call method: %s\n", strerror(-r));
> +            return -1;
> +        }
> +    }
>      else if (reqptr->parameter == 6) // Subnet
>      {
>          sprintf(new_netmask, "%d.%d.%d.%d", reqptr->data[0], reqptr->data[1], reqptr->data[2], reqptr->data[3]);
> --
> 2.6.4
>
>
> _______________________________________________
> openbmc mailing list
> openbmc at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc