On 02/05/2016 11:49 AM, Ján Tomko wrote:
> For the actions ADD and OLD, split out creating the new lease object,
> as well as getting the environment variables that do not affect
> the parsing of command line arguments.
> ---
> v2: do not pass argc, argv to the function
>
> src/network/leaseshelper.c | 127 +++++++++++++++++++++++++++------------------
> 1 file changed, 77 insertions(+), 50 deletions(-)
>
> diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
> index f560206..2a0b18c 100644
> --- a/src/network/leaseshelper.c
> +++ b/src/network/leaseshelper.c
> @@ -285,10 +285,82 @@ virLeasePrintLeases(virJSONValuePtr leases_array_new,
> return ret;
> }
>
> +static int
> +virLeaseNew(virJSONValuePtr *lease_ret,
> + const char *mac,
> + const char *clientid,
> + const char *ip,
> + const char *hostname,
> + const char *iaid,
> + const char *server_duid)
Coverity complains - order of arguments appears swapped...
...iaid, server_duid)
[1]
> +{
> + virJSONValuePtr lease_new = NULL;
> + const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES");
> + long long expirytime = 0;
> + char *exptime = NULL;
> + int ret = -1;
> +
> + /* In case hostname is still unknown, use the last known one */
> + if (!hostname)
> + hostname = virGetEnvAllowSUID("DNSMASQ_OLD_HOSTNAME");
> +
> + if (!mac) {
> + ret = 0;
> + goto cleanup;
> + }
> +
> + if (exptime_tmp) {
> + if (VIR_STRDUP(exptime, exptime_tmp) < 0)
> + goto cleanup;
> +
> + /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES
> + * (dnsmasq < 2.52) */
> + if (exptime[strlen(exptime) - 1] == ' ')
> + exptime[strlen(exptime) - 1] = '\0';
> + }
> +
> + if (!exptime ||
> + virStrToLong_ll(exptime, NULL, 10, &expirytime) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to convert lease expiry time to long long: %s"),
> + NULLSTR(exptime));
> + goto cleanup;
> + }
> +
> + /* Create new lease */
> + if (!(lease_new = virJSONValueNewObject())) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to create json"));
> + goto cleanup;
> + }
> +
> + if (iaid && virJSONValueObjectAppendString(lease_new, "iaid", iaid) < 0)
> + goto cleanup;
> + if (ip && virJSONValueObjectAppendString(lease_new, "ip-address", ip) < 0)
> + goto cleanup;
> + if (mac && virJSONValueObjectAppendString(lease_new, "mac-address", mac) < 0)
> + goto cleanup;
> + if (hostname && virJSONValueObjectAppendString(lease_new, "hostname", hostname) < 0)
> + goto cleanup;
> + if (clientid && virJSONValueObjectAppendString(lease_new, "client-id", clientid) < 0)
> + goto cleanup;
> + if (server_duid && virJSONValueObjectAppendString(lease_new, "server-duid", server_duid) < 0)
> + goto cleanup;
> + if (expirytime && virJSONValueObjectAppendNumberLong(lease_new, "expiry-time", expirytime) < 0)
> + goto cleanup;
> +
> + ret = 0;
> + *lease_ret = lease_new;
> + lease_new = NULL;
> + cleanup:
> + VIR_FREE(exptime);
> + virJSONValueFree(lease_new);
> + return ret;
> +}
> +
> int
> main(int argc, char **argv)
> {
> - char *exptime = NULL;
> char *pid_file = NULL;
> char *custom_lease_file = NULL;
> const char *ip = NULL;
> @@ -297,10 +369,8 @@ main(int argc, char **argv)
> const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID");
> const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID");
> const char *interface = virGetEnvAllowSUID("DNSMASQ_INTERFACE");
> - const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES");
> const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME");
> char *server_duid = NULL;
> - long long expirytime = 0;
> int action = -1;
> int pid_file_fd = -1;
> int rv = EXIT_FAILURE;
> @@ -362,20 +432,6 @@ main(int argc, char **argv)
> if (argc == 5)
> hostname = argv[4];
>
> - /* In case hostname is still unknown, use the last known one */
> - if (!hostname)
> - hostname = virGetEnvAllowSUID("DNSMASQ_OLD_HOSTNAME");
> -
> - if (exptime_tmp) {
> - if (VIR_STRDUP(exptime, exptime_tmp) < 0)
> - goto cleanup;
> -
> - /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES
> - * (dnsmasq < 2.52) */
> - if (exptime[strlen(exptime) - 1] == ' ')
> - exptime[strlen(exptime) - 1] = '\0';
> - }
> -
> /* Check if it is an IPv6 lease */
> if (iaid) {
> mac = virGetEnvAllowSUID("DNSMASQ_MAC");
> @@ -405,6 +461,9 @@ main(int argc, char **argv)
> switch ((enum virLeaseActionFlags) action) {
> case VIR_LEASE_ACTION_ADD:
> case VIR_LEASE_ACTION_OLD:
> + /* Create new lease */
> + if (virLeaseNew(&lease_new, mac, clientid, ip, hostname, server_duid, iaid) < 0)
While here it's ...server_duid, iaid)
John
> + goto cleanup;
> /* Custom ipv6 leases *will not* be created if the env-var DNSMASQ_MAC
> * is not set. In the special case, when the $(interface).status file
> * is not already present and dnsmasq is (re)started, the corresponding
> @@ -418,40 +477,9 @@ main(int argc, char **argv)
> * the new lease will be created irrespective of whether the MACID is
> * known or not.
> */
> - if (!mac)
> + if (!lease_new)
> break;
>
> - /* Create new lease */
> - if (!(lease_new = virJSONValueNewObject())) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("failed to create json"));
> - goto cleanup;
> - }
> -
> - if (!exptime ||
> - virStrToLong_ll(exptime, NULL, 10, &expirytime) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Unable to convert lease expiry time to long long: %s"),
> - NULLSTR(exptime));
> - goto cleanup;
> - }
> -
> - if (iaid && virJSONValueObjectAppendString(lease_new, "iaid", iaid) < 0)
> - goto cleanup;
> - if (ip && virJSONValueObjectAppendString(lease_new, "ip-address", ip) < 0)
> - goto cleanup;
> - if (mac && virJSONValueObjectAppendString(lease_new, "mac-address", mac) < 0)
> - goto cleanup;
> - if (hostname && virJSONValueObjectAppendString(lease_new, "hostname", hostname) < 0)
> - goto cleanup;
> - if (clientid && virJSONValueObjectAppendString(lease_new, "client-id", clientid) < 0)
> - goto cleanup;
> - if (server_duid && virJSONValueObjectAppendString(lease_new, "server-duid", server_duid) < 0)
> - goto cleanup;
> - if (expirytime && virJSONValueObjectAppendNumberLong(lease_new, "expiry-time", expirytime) < 0)
> - goto cleanup;
> -
> -
> /* fallthrough */
> case VIR_LEASE_ACTION_DEL:
> /* Delete the corresponding lease, if it already exists */
> @@ -514,7 +542,6 @@ main(int argc, char **argv)
> virPidFileReleasePath(pid_file, pid_file_fd);
>
> VIR_FREE(pid_file);
> - VIR_FREE(exptime);
> VIR_FREE(server_duid);
> VIR_FREE(custom_lease_file);
> virJSONValueFree(lease_new);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list