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...
free() handles a NULL pointer by doing nothing, and it will behave this way on any posix system compliant system. However, on an OpenBSD system you could have actually read the documentation to check this. > I stand by my argument that there's no clear improvement, especially > on a modern system. In my opinion it's marginally simpler to read and less likely to result in a future cut-n-paste error. > 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; > >> } > >> > > > > > > > > -- > > Aaron Mason - Programmer, open source addict > > I've taken my software vows - for beta or for worse > > > > -- > Aaron Mason - Programmer, open source addict > I've taken my software vows - for beta or for worse
