You can enable a bunch of warnings with WARNINGS=Yes in our tree. On Dec 28, 2012 3:34 PM, "Kenneth R Westerback" <[email protected]> wrote:
> On Thu, Dec 27, 2012 at 02:04:24PM +0100, Maxime Villard wrote: > > Well, as no one seems to give a fuck on tech@, I put a more > > glamourous title here. > > > > btw, i wonder why you don't put -Wextra to the makefile, you would > > see that there are a lot of unused parameters, comparisons between > > signed and unsigned, uninitialized vars, ... > > Too many false positives to be useful. > > People are running various static analysis packages over the tree > periodically to find such things. None are perfect so if you actually > find any you think are bugs, diffs are always appreciated. > > .... Ken > > > > > > > -------- Message original -------- > > Sujet: [PATCH] pfctl: leak & stuff > > Date : Sat, 22 Dec 2012 08:16:09 +0100 > > De : Maxime Villard <[email protected]> > > Pour : [email protected] > > > > 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; > > }
