Re: [libvirt] [PATCH 2/4] net-dhcp-leases: Implement the remote protocol

11 Sep 2013

      ...
+
+    ret->leases.leases_len = nleases;
+
+    for (i = 0; i < nleases; i++) {
+        virNetworkDHCPLeasesPtr lease = leases[i];
+        remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
+        lease_ret->expirytime = lease->expirytime;
+
+        if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||
+            (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||
+            (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||
+            (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))
+            goto error;
+    }
+
This above code is reported so many times in the patch, in
networkGetDHCPLeaseForMAC, remoteNetworkGetDHCPLeaseForMAC,
remoteNetworkGetDHCPLeases, remoteDispatchNetworkGetDHCPLeaseForMAC
and remoteSerializeNetworkDHCPLeases.

Hence I was wondering, whether it would be useful to introduce a helper
function, say virNetworkDHCPLeaseCopy, which would copy lease contents
from one lease pointer to another, something like:
/**
 * virNetworkDHCPLeaseCopy:
 * @dst: pointer to the destination lease object
 * @src: pointer to the source lease object
 *
 * Copies contents of @src to @dst.
 *
 * Returns 0 on success, -1 otherwise.
 */
int
virNetworkDHCPLeaseCopy(virNetworkDHCPLeasesPtr dst,
                        virNetworkDHCPLeasesPtr src)
{
    dst->expirytime = src->expirytime;
    if ((VIR_STRDUP(dst->macaddr, src->macaddr) < 0) ||
        (VIR_STRDUP(dst->ipaddr, src->ipaddr) < 0) ||
        (VIR_STRDUP(dst->hostname, src->hostname) < 0) ||
        (VIR_STRDUP(dst->clientid, src->clientid) < 0))
        return -1;
    return 0;
}

But the issue with this is that it wants the user to allocate memory
to the dst pointer (As the code in remote protocols does).
Also, will it be ever used by the users? If not, then should we be
introducing this function to some place in /src/util, so that
code redundancy can be removed?

-- 
Nehal J Wani