[PATCH] pfctl: leak & stuff

[Maxime Villard]

Hi,
here are my small changes for pfctl.

1) There are cases where we could leak a file descriptor by
   returning.

2) We don't need to check memory before freeing it, as
   free() already does that.

3) Just replaced a snprintf() by strlcpy(), it's faster.

Ok/Comments ?



Index: pfctl.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.314
diff -u -r1.314 pfctl.c
--- pfctl.c     19 Sep 2012 15:52:17 -0000      1.314
+++ pfctl.c     22 Dec 2012 07:08:28 -0000
@@ -1377,8 +1377,7 @@
                                err(1, "DIOCXROLLBACK");
                exit(1);
        } else {                /* sub ruleset */
-               if (path)
-                       free(path);
+               free(path);
                return (-1);
        }
 
@@ -1867,10 +1866,6 @@
        unsigned int len = 0;
        size_t n;
 
-       f = fopen(file, "w");
-       if (f == NULL)
-               err(1, "open: %s", file);
-
        memset(&ps, 0, sizeof(ps));
        for (;;) {
                ps.ps_len = len;
@@ -1893,6 +1888,10 @@
                        return; /* no states */
                len *= 2;
        }
+
+       f = fopen(file, "w");
+       if (f == NULL)
+               err(1, "open: %s", file);
 
        n = ps.ps_len / sizeof(struct pfsync_state);
        if (fwrite(inbuf, sizeof(struct pfsync_state), n, f) < n)
Index: pfctl_osfp.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl_osfp.c,v
retrieving revision 1.18
diff -u -r1.18 pfctl_osfp.c
--- pfctl_osfp.c        18 Oct 2010 15:55:28 -0000      1.18
+++ pfctl_osfp.c        22 Dec 2012 07:08:28 -0000
@@ -112,16 +112,11 @@
 
        while ((line = fgetln(in, &len)) != NULL) {
                lineno++;
-               if (class)
-                       free(class);
-               if (version)
-                       free(version);
-               if (subtype)
-                       free(subtype);
-               if (desc)
-                       free(desc);
-               if (tcpopts)
-                       free(tcpopts);
+               free(class);
+               free(version);
+               free(subtype);
+               free(desc);
+               free(tcpopts);
                class = version = subtype = desc = tcpopts = NULL;
                memset(&fp, 0, sizeof(fp));
 
@@ -250,16 +245,11 @@
                add_fingerprint(dev, opts, &fp);
        }
 
-       if (class)
-               free(class);
-       if (version)
-               free(version);
-       if (subtype)
-               free(subtype);
-       if (desc)
-               free(desc);
-       if (tcpopts)
-               free(tcpopts);
+       free(class);
+       free(version);
+       free(subtype);
+       free(desc);
+       free(tcpopts);
 
        fclose(in);
 
@@ -513,7 +503,7 @@
        return (buf);
 
 found:
-       snprintf(buf, len, "%s", class_name);
+       strlcpy(buf, class_name, len);
        if (version_name) {
                strlcat(buf, " ", len);
                strlcat(buf, version_name, len);
Index: pfctl_radix.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl_radix.c,v
retrieving revision 1.29
diff -u -r1.29 pfctl_radix.c
--- pfctl_radix.c       27 Jul 2011 00:26:10 -0000      1.29
+++ pfctl_radix.c       22 Dec 2012 07:08:28 -0000
@@ -499,8 +499,7 @@
 {
        if (b == NULL)
                return;
-       if (b->pfrb_caddr != NULL)
-               free(b->pfrb_caddr);
+       free(b->pfrb_caddr);
        b->pfrb_caddr = NULL;
        b->pfrb_size = b->pfrb_msize = 0;
 }