On Mon, Dec 31, 2012 at 04:53:15PM +1100, Aaron Mason wrote: > Ok, I just tried freeing NULL, and it did nothing. Granted it was on > a Linux system but still... > > I stand by my argument that there's no clear improvement, especially > on a modern system.
It's less code, and code gets copied, and people don't read docs, they copy code and cross their fingers, we shouldn't incentivate that. > > On Mon, Dec 31, 2012 at 4:50 PM, Aaron Mason <[email protected]> > wrote: > > Maxime > > > > I'm not entirely clear on what you hoped to achieve with the diffs > > below, if anything you're inducing possible segfaults if any of those > > values are NULL. That aside, I fail to see how this could be > > construed as any sort of improvement. > > > >> 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; > >> }
