Re: CVS commit: src/usr.sbin/envstat
Module Name:src
Committed By: christos
Date: Thu Dec 13 19:31:25 UTC 2012
Modified Files:
src/usr.sbin/envstat: envstat.c
Log Message:
PR/47316: Henning Petersen: Memory leak in envstat with config file.
While we're making sure to free() things, should we not also defend
against memory leaks in the case where an option is used more than once?
RCS file: /cvsroot/src/usr.sbin/envstat/envstat.c,v
retrieving revision 1.92
diff -u -p -r1.92 envstat.c
--- envstat.c 13 Dec 2012 19:31:25 - 1.92
+++ envstat.c 13 Dec 2012 19:47:44 -
@@ -132,6 +132,7 @@ int main(int argc, char **argv)
while ((c = getopt(argc, argv, c:Dd:fIi:klrSs:Tw:Wx)) != -1) {
switch (c) {
case 'c': /* configuration file */
+ free(configfile);
configfile = strdup(optarg);
if (configfile == NULL)
err(EXIT_FAILURE, strdup);
@@ -140,6 +141,7 @@ int main(int argc, char **argv)
flags |= ENVSYS_DFLAG;
break;
case 'd': /* show sensors of a specific device */
+ free(mydevname);
mydevname = strdup(optarg);
if (mydevname == NULL)
err(EXIT_FAILURE, strdup);
@@ -171,6 +173,7 @@ int main(int argc, char **argv)
flags |= ENVSYS_SFLAG;
break;
case 's': /* only show specified sensors */
+ free(sensors);
sensors = strdup(optarg);
if (sensors == NULL)
err(EXIT_FAILURE, strdup);
-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-
Re: CVS commit: src/usr.sbin/envstat
On Thu, Dec 13, 2012 at 11:53:24AM -0800, Paul Goyette wrote: While we're making sure to free() things, should we not also defend against memory leaks in the case where an option is used more than once? While we are here, I wonder why sysmon(9) does not follow the common guidelines w.r.t. for instance queue(3). I know quite well that none of this is your making, but smalll refactorings like this could make the code more understandable to other people. - Jukka.
Re: CVS commit: src/usr.sbin/envstat
On Mar 31, 12:00am, Paul Goyette wrote:
}
} Module Name:src
} Committed By: christos
} Date: Thu Dec 13 19:31:25 UTC 2012
}
} Modified Files:
} src/usr.sbin/envstat: envstat.c
}
} Log Message:
} PR/47316: Henning Petersen: Memory leak in envstat with config file.
}
} While we're making sure to free() things, should we not also defend
} against memory leaks in the case where an option is used more than once?
}
} RCS file: /cvsroot/src/usr.sbin/envstat/envstat.c,v
} retrieving revision 1.92
} diff -u -p -r1.92 envstat.c
} --- envstat.c 13 Dec 2012 19:31:25 - 1.92
} +++ envstat.c 13 Dec 2012 19:47:44 -
} @@ -132,6 +132,7 @@ int main(int argc, char **argv)
} while ((c = getopt(argc, argv, c:Dd:fIi:klrSs:Tw:Wx)) != -1) {
} switch (c) {
} case 'c': /* configuration file */
} + free(configfile);
} configfile = strdup(optarg);
} if (configfile == NULL)
} err(EXIT_FAILURE, strdup);
If you're going to be this paranoid, you should make sure that
you're not passing NULL to free(3). That can blow up on some systems.
Anyways, I see that Christos has already fixed this in a different way,
so it doesn't really matter.
}-- End of excerpt from Paul Goyette
Re: CVS commit: src/usr.sbin/envstat
On Dec 13, 2012, at 4:30 PM, John Nemeth wrote:
On Mar 31, 12:00am, Paul Goyette wrote:
}
} Module Name:src
} Committed By: christos
} Date: Thu Dec 13 19:31:25 UTC 2012
}
} Modified Files:
} src/usr.sbin/envstat: envstat.c
}
} Log Message:
} PR/47316: Henning Petersen: Memory leak in envstat with config file.
}
} While we're making sure to free() things, should we not also defend
} against memory leaks in the case where an option is used more than once?
}
} RCS file: /cvsroot/src/usr.sbin/envstat/envstat.c,v
} retrieving revision 1.92
} diff -u -p -r1.92 envstat.c
} --- envstat.c 13 Dec 2012 19:31:25 - 1.92
} +++ envstat.c 13 Dec 2012 19:47:44 -
} @@ -132,6 +132,7 @@ int main(int argc, char **argv)
} while ((c = getopt(argc, argv, c:Dd:fIi:klrSs:Tw:Wx)) != -1) {
} switch (c) {
} case 'c': /* configuration file */
} + free(configfile);
} configfile = strdup(optarg);
} if (configfile == NULL)
} err(EXIT_FAILURE, strdup);
If you're going to be this paranoid, you should make sure that
you're not passing NULL to free(3). That can blow up on some systems.
Anyways, I see that Christos has already fixed this in a different way,
so it doesn't really matter.
free(NULL); has been required to be a nop since c89.
Warner
Re: CVS commit: src/usr.sbin/envstat
On Fri, 14 Dec 2012, Jukka Ruohonen wrote: On Thu, Dec 13, 2012 at 11:53:24AM -0800, Paul Goyette wrote: While we're making sure to free() things, should we not also defend against memory leaks in the case where an option is used more than once? While we are here, I wonder why sysmon(9) does not follow the common guidelines w.r.t. for instance queue(3). I know quite well that none of this is your making, but smalll refactorings like this could make the code more understandable to other people. There's lots of clean-up work to do here. Yes, it needs to be done, but I just haven't had the time or energy to dive into it deep enough! I promise, I will get to it. I just can't promise when. :) - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/usr.sbin/envstat
It seems that the following commit has introduced a regression for the dev/sysmon/t_swsensor atf tests (for details, see test results at http://screamer.whooppee.com/amd64-results/4722_1_atf.html) Module Name:src Committed By: christos Date: Thu Dec 13 20:06:42 UTC 2012 Modified Files: src/usr.sbin/envstat: envstat.c Log Message: - no point in allocating memory to hold command line arguments. - allocate memory inside the function used. I'll take a look and see what happened. The tests should get fixed fairly soon. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
