<div dir="ltr"><br><br>> +static int<br>> +remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,<br>> +                                 int nleases,<br>> +                                 remote_network_get_dhcp_leases_ret *ret)<br>
> +{<br>> +    size_t i;<br>> +<br>> +    if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) {<br>> +        virReportError(VIR_ERR_INTERNAL_ERROR,<br>> +                       _("Number of leases is %d, which exceeds max limit: %d"),<br>
> +                       nleases, REMOTE_NETWORK_DHCP_LEASES_MAX);<br>> +        return -1;<br>> +    }<br>> +<br>> +    if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)<br>> +        return -1;<br>
> +<br>> +    ret->leases.leases_len = nleases;<br>> +<br>> +    for (i = 0; i < nleases; i++) {<br>> +        virNetworkDHCPLeasesPtr lease = leases[i];<br>> +        remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);<br>
> +        lease_ret->expirytime = lease->expirytime;<br>> +        lease_ret->type = lease->type;<br>> +        lease_ret->prefix = lease->prefix;<br>> +<br>> +        if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||<br>
> +            (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||<br>> +            (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||<br>> +            (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))<br>
> +            goto error;<br>> +    }<br>> +<br>> +    return 0;<br>> +<br>> +error:<br>> +    for (i = 0; i < nleases; i++) {<br>> +        remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);<br>
> +        virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);<div><br><div>[1]</div><div><br><div>> +    }<br>> +    return -1;<br>> +}<br>> +<br>> +static int<br>> +remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,<br>
> +                                   virNetServerClientPtr client,<br>> +                                   virNetMessagePtr msg ATTRIBUTE_UNUSED,<br>> +                                   virNetMessageErrorPtr rerr,<br>
> +                                   remote_network_get_dhcp_leases_args *args,<br>> +                                   remote_network_get_dhcp_leases_ret *ret)<br>> +{<br>> +    int rv = -1;<br>> +    struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);<br>
> +    virNetworkDHCPLeasesPtr *leases = NULL;<br>> +    virNetworkPtr net = NULL;<br>> +    int nleases = 0;<br>> +<br>> +    if (!priv->conn) {<br>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));<br>
> +        goto cleanup;<br>> +    }<br>> +<br>> +    if (!(net = get_nonnull_network(priv->conn, args->net)))<br>> +        goto cleanup;<br>> +<br>> +    if ((nleases = virNetworkGetDHCPLeases(net, &leases, args->flags)) < 0)<br>
> +        goto cleanup;<br>> +<br>> +    if (remoteSerializeNetworkDHCPLeases(leases, nleases, ret) < 0)<br>> +        goto cleanup;<br>><br>> +    rv = nleases;<br>> +<br>> +cleanup:<br>> +    if (rv < 0)<br>
> +        virNetMessageSaveError(rerr);<br>> +    if (leases) {<br>> +        size_t i;<br>> +        for (i = 0; i < nleases; i++) {<br>> +            virNetworkDHCPLeaseFree(leases[i]);<br>> +        }<br>
> +    }<br>> +    VIR_FREE(leases);<br>> +    virNetworkFree(net);<br>> +    return rv;<br>> +}<br>> +<br>> +static int<br>> +remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server ATTRIBUTE_UNUSED,<br>
> +                                         virNetServerClientPtr client,<br>> +                                         virNetMessagePtr msg ATTRIBUTE_UNUSED,<br>> +                                         virNetMessageErrorPtr rerr,<br>
> +                                         remote_network_get_dhcp_leases_for_mac_args *args,<br>> +                                         remote_network_get_dhcp_leases_for_mac_ret *ret)<br>> +{<br>> +    int rv = -1;<br>
> +    struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);<br>> +    virNetworkDHCPLeasesPtr *leases = NULL;<br>> +    virNetworkPtr net = NULL;<br>> +    int nleases = 0;<br>> +    const char *mac = NULL;<br>
> +<br>> +    if (!priv->conn) {<br>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));<br>> +        goto cleanup;<br>> +    }<br>> +<br>> +    if (!(net = get_nonnull_network(priv->conn, args->net)))<br>
> +        goto cleanup;<br>> +<br>> +    mac = args->mac ? *args->mac : NULL;<br>> +<br>> +    if ((nleases = virNetworkGetDHCPLeasesForMAC(net, mac, &leases, args->flags)) < 0)<br>> +        goto cleanup;<br>
> +<br>> +    if (remoteSerializeNetworkDHCPLeases(leases, nleases,<br>> +                                         (remote_network_get_dhcp_leases_ret *)ret) < 0)</div><div>[2]</div><div><br>> +        goto cleanup;<br>
> +<br>> +    rv = nleases;<br>> +<br><br></div></div></div><div><br></div><div>I am a little skeptical about the typecasts involved in [1] and [2]</div><div><br></div><div>Specifically for [2], although the APIs are different, the struct is same,</div>
<div>only the name is different. But, what would be the other options?</div><div><br></div><div>One option: (Suggested by Osier)</div><div>Change <span style="font-size:13px;font-family:arial,sans-serif">remoteSerializeNetworkDHCPLeas</span><span style="font-size:13px;font-family:arial,sans-serif">es to</span></div>
<div><span style="font-size:13px;font-family:arial,sans-serif"><br></span></div><div><span style="font-size:13px;font-family:arial,sans-serif">+</span><span style="font-size:13px;font-family:arial,sans-serif">remoteSerializeNetworkDHCPLeas</span><span style="font-size:13px;font-family:arial,sans-serif">es(virNetworkDHCPLeasesPtr *leases,</span></div>
<span style="font-family:arial,sans-serif;font-size:13px">+                                 int nleases,</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">+                                 (void </span><span style="font-family:arial,sans-serif;font-size:13px">*) ret,</span><div>
<span style="font-size:13px;font-family:arial,sans-serif">+                                 int method)</span><br></div><div><span style="font-size:13px;font-family:arial,sans-serif"><br></span></div><div><span style="font-size:13px;font-family:arial,sans-serif">where the datatype for variable in which ret will be copied, will be</span></div>
<div><span style="font-size:13px;font-family:arial,sans-serif">decided based on the method (method's value will be taken from an enum</span></div><div><span style="font-size:13px;font-family:arial,sans-serif">or something similar, e.g: 1->DHCPLeases, 2->DHCPLeasesForMAC)</span></div>
<div><span style="font-size:13px;font-family:arial,sans-serif"><br></span></div><div><span style="font-size:13px;font-family:arial,sans-serif">OR</span></div><div><span style="font-size:13px;font-family:arial,sans-serif"><br>
</span></div><div><span style="font-size:13px;font-family:arial,sans-serif">Second Option: Since the APIs should be independent, make another function:</span></div><div><font face="arial, sans-serif">remoteSerializeNetworkDHCPLeasesForMAC(...), but that will lead to code</font><br>
</div><div><font face="arial, sans-serif">redundancy.</font></div><div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">Eric, what would you do in these cases?</font></div></div>