Additional patch that reduces some repeating parts.
On 10/25/2018 10:36 AM, Petr Mensik wrote:
> Hi again.
>
> This time I have a little bit more controversal patches. But I think
> still useful. They fixes memory leaks that might occur in some cases.
> Most dnsmasq errors is fatal, so it does not matter. But some are not.
> Some parts are reloaded on SIGHUP signal, so it might leak more than once.
>
> Some example when it changes the failures. Use dhcp-options file with
> this content:
>
> tag:error,vendor:redhat
> option:ntp-server,1.2.3.4.5
> option6:ntp-server,[:::]
>
> Is not fatal and dnsmasq will start. On each reload command, it would
> leak some memory. I validated it using valgrind --leak-check=full
> dnsmasq -d. This patch fixes it. It introduces something that might be
> considered constructor and destructor of selected structures. What do
> you think of it?
>
> Comments are welcome. Another patch would be sent short after, they are
> too big together to require moderator attention.
>
> Cheers,
> Petr
>
>
>
> ___
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss@lists.thekelleys.org.uk
> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
>
--
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com PGP: 65C6C973
>From ae3837cabc7e7b2fbd2d875f403a3f26a4d81422 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?=
Date: Thu, 16 Aug 2018 18:10:25 +0200
Subject: [PATCH 2/2] Reduce repeating code parsing tags
DHCP parameters often can have optional tags before it. Reduce repeated
parsing of it, use dhcp_tags() to parse and free if unsuccessful.
Make sure null pointer will not crash free function.
Free also errors in dhcp-range.
---
src/option.c | 326 +++
1 file changed, 171 insertions(+), 155 deletions(-)
diff --git a/src/option.c b/src/option.c
index 547d36e..66847fb 100644
--- a/src/option.c
+++ b/src/option.c
@@ -577,14 +577,15 @@ static void *opt_malloc(size_t size)
return ret;
}
-static char *opt_string_alloc(char *cp)
+static char *opt_string_alloc(const char *cp)
{
char *ret = NULL;
+ size_t len;
- if (cp && strlen(cp) != 0)
+ if (cp && (len = strlen(cp)) != 0)
{
- ret = opt_malloc(strlen(cp)+1);
- strcpy(ret, cp);
+ ret = opt_malloc(len+1);
+ memcpy(ret, cp, len+1);
/* restore hidden metachars */
unhide_metas(ret);
@@ -760,6 +761,7 @@ static void do_usage(void)
#define ret_err(x) do { strcpy(errstr, (x)); return 0; } while (0)
#define ret_err_free(x,m) do { strcpy(errstr, (x)); free((m)); return 0; } while (0)
+#define goto_err(x) do { strcpy(errstr, (x)); goto on_error; } while (0)
static char *parse_mysockaddr(char *arg, union mysockaddr *addr)
{
@@ -961,6 +963,97 @@ static char *set_prefix(char *arg)
return arg;
}
+static struct dhcp_netid *
+dhcp_netid_create(const char *net, struct dhcp_netid *next)
+{
+ struct dhcp_netid *tt;
+ tt = opt_malloc(sizeof (struct dhcp_netid));
+ tt->net = opt_string_alloc(net);
+ tt->next = next;
+ return tt;
+}
+
+static void dhcp_netid_free(struct dhcp_netid *nid)
+{
+ while (nid)
+{
+ struct dhcp_netid *tmp = nid;
+ nid = nid->next;
+ free(tmp->net);
+ free(tmp);
+}
+}
+
+/* Parse one or more tag:s before parameters.
+ * Moves arg to the end of tags. */
+static struct dhcp_netid * dhcp_tags(char **arg)
+{
+ struct dhcp_netid *id = NULL;
+
+ while (is_tag_prefix(*arg))
+{
+ char *comma = split(*arg);
+ id = dhcp_netid_create((*arg)+4, id);
+ *arg = comma;
+};
+ if (!*arg)
+{
+ dhcp_netid_free(id);
+ id = NULL;
+}
+ return id;
+}
+
+static void dhcp_netid_list_free(struct dhcp_netid_list *netid)
+{
+ while (netid)
+{
+ struct dhcp_netid_list *tmplist = netid;
+ netid = netid->next;
+ dhcp_netid_free(tmplist->list);
+ free(tmplist);
+}
+}
+
+static void dhcp_config_free(struct dhcp_config *config)
+{
+ if (config)
+{
+ struct hwaddr_config *hwaddr = config->hwaddr;
+ while (hwaddr)
+{
+ struct hwaddr_config *tmp = hwaddr;
+ hwaddr = hwaddr->next;
+ free(tmp);
+}
+ dhcp_netid_list_free(config->netid);
+ if (config->flags & CONFIG_CLID)
+free(config->clid);
+ free(config);
+}
+}
+
+static void dhcp_context_free(struct dhcp_context *ctx)
+{
+ if (ctx)
+{
+ dhcp_netid_free(ctx->filter);
+ free(ctx->netid.net);
+ free(ctx->template_interface);
+ free(ctx);
+}
+}
+
+static void dhcp_opt_free(struct dhcp_opt *opt)
+{
+ if (opt->flags & DHOPT_VENDOR)
+free(opt->u.vendor_class);
+ dhcp_netid_free(opt->netid);
+ free(opt->val);
+ free(opt);
+}
+
+
/* This is too insanely large to keep in-line in the switch */
static int parse_dhcp_opt(char *errstr, char *arg, int flags)
{
@@ -968,7 +1061,6 @@ static
