<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>