CVS commit: src/usr.sbin/envstat
Module Name:src
Committed By: rin
Date: Mon Jun 19 03:03:11 UTC 2023
Modified Files:
src/usr.sbin/envstat: envstat.c
Log Message:
Silence wrong maybe-uninitialized raised by GCC/x86_64 10.4.0 -Os.
To generate a diff of this commit:
cvs rdiff -u -r1.103 -r1.104 src/usr.sbin/envstat/envstat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.sbin/envstat/envstat.c
diff -u src/usr.sbin/envstat/envstat.c:1.103 src/usr.sbin/envstat/envstat.c:1.104
--- src/usr.sbin/envstat/envstat.c:1.103 Mon Nov 21 21:24:02 2022
+++ src/usr.sbin/envstat/envstat.c Mon Jun 19 03:03:11 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: envstat.c,v 1.103 2022/11/21 21:24:02 brad Exp $ */
+/* $NetBSD: envstat.c,v 1.104 2023/06/19 03:03:11 rin Exp $ */
/*-
* Copyright (c) 2007, 2008 Juan Romero Pardines.
@@ -27,7 +27,7 @@
#include
#ifndef lint
-__RCSID("$NetBSD: envstat.c,v 1.103 2022/11/21 21:24:02 brad Exp $");
+__RCSID("$NetBSD: envstat.c,v 1.104 2023/06/19 03:03:11 rin Exp $");
#endif /* not lint */
#include
@@ -236,7 +236,8 @@ int main(int argc, char **argv)
err(EXIT_FAILURE, "add_sensors");
}
if (sensors) {
-char *dvstring, *sstring, *p, *last, *s;
+char *sstring, *p, *last, *s;
+char *dvstring = NULL; /* XXXGCC */
unsigned count = 0;
s = strdup(sensors);
CVS commit: src/usr.sbin/envstat
Module Name:src Committed By: rin Date: Mon Jun 19 03:03:11 UTC 2023 Modified Files: src/usr.sbin/envstat: envstat.c Log Message: Silence wrong maybe-uninitialized raised by GCC/x86_64 10.4.0 -Os. To generate a diff of this commit: cvs rdiff -u -r1.103 -r1.104 src/usr.sbin/envstat/envstat.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/usr.sbin/envstat
On Sun, 16 Dec 2012, David Holland wrote: On Fri, Dec 14, 2012 at 06:18:57PM +0100, Marc Balmer wrote: proper resource management is a good thing - if the code continues to run. In this case, where the program exits, there no benefit from freeing up memory etc. Buncombe. is that a technical argument? I didn't understand it, in any case.. can you explain why, in C, when exit() explicitly closes all open file descriptors and releases any memory etc (see a detailed list in exit(3) and _exit(3)), there might be a benefit in doing so formally before calling exit() ? regards, iain
Re: CVS commit: src/usr.sbin/envstat
In article alpine.neb.2.00.1212171106140.2...@galant.ogmig.net, Iain Hibbert plu...@ogmig.net wrote: I didn't understand it, in any case.. can you explain why, in C, when exit() explicitly closes all open file descriptors and releases any memory etc (see a detailed list in exit(3) and _exit(3)), there might be a benefit in doing so formally before calling exit() ? It is a slippery slope. Every program eventually exits, so it is up to the author of the code to decide when it is late enough to avoid doing cleanup. I think that everyone agrees that libraries should have as few side effects as possible and free resources when they are not needed. In the application case, this is not so clear. There are claims of: - loss of readabily - slowing down exit - the OS does not free resources until process destruction anyway On the other side there is: - memory usage/leak tools complain - destruction path gets written/tested - code gets reused/refactored in different places, and then the leaks become apparent - sloppiness and laziness is not good programming practice - buncombe :-) And don't know, are there any others? My experience has been that in the past, when I had to add resource deallocation on code that did not do it before, it has been a complex, time consuming, and error prone task. The main reason for that was that the people who designed the code relied on the fact that there would be no resource deallocation. This led to bad coding practices throughout the code (why keep the original pointer around when we are not planning to free it anyway; we can just move it around). All these design decisions required heavy-handed structural changes to the code to make it use resources properly. So, I design for code reuse; I like to be able to use memory leak detectors, and I've had to deal with programs that did not do so well about resource allocation in the past (make, csh) so I appreciate code that is designed with deallocation in mind. christos
Re: CVS commit: src/usr.sbin/envstat
On Fri, Dec 14, 2012 at 06:18:57PM +0100, Marc Balmer wrote: Similarly it just isn't worth trying to free resources prior to program exit. Have you ever waited while a big C++ program runs all its destructors, paging in code and data just to exit! Depends, it makes tools like valgrind a lot easier to use. Source code should be optimized for readability, performance, stability, and not for debug tools. Ironically, proper ressource managements tends to make it much easier to fulfill at least two of the three items... proper resource management is a good thing - if the code continues to run. In this case, where the program exits, there no benefit from freeing up memory etc. Buncombe. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/usr.sbin/envstat
On Thu, Dec 13, 2012 at 08:45:02PM -0800, Paul Goyette wrote: 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. If you are worried about malloc() fails then (maybe) allocating memory when parsing the command line would make sense - since any fail is then guaranteed to happen before any processing. OTOH malloc() is unlikely to fail for small programs unless someone is testing whether the program survives malloc failure. Similarly it just isn't worth trying to free resources prior to program exit. Have you ever waited while a big C++ program runs all its destructors, paging in code and data just to exit! About the only environment wheere is matters is when programs are run as shell builtins - and that will always be a small subset of programs. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/usr.sbin/envstat
On Fri, Dec 14, 2012 at 08:59:37AM +, David Laight wrote: Similarly it just isn't worth trying to free resources prior to program exit. Have you ever waited while a big C++ program runs all its destructors, paging in code and data just to exit! Depends, it makes tools like valgrind a lot easier to use. Joerg
Re: CVS commit: src/usr.sbin/envstat
Am 14.12.2012 um 17:07 schrieb Joerg Sonnenberger jo...@britannica.bec.de: On Fri, Dec 14, 2012 at 08:59:37AM +, David Laight wrote: Similarly it just isn't worth trying to free resources prior to program exit. Have you ever waited while a big C++ program runs all its destructors, paging in code and data just to exit! Depends, it makes tools like valgrind a lot easier to use. Source code should be optimized for readability, performance, stability, and not for debug tools. And that means to consider realities like the fact that on Unix resources are freed when the process exits and an explicit free is counter productive if terms of effectiveness. Joerg
Re: CVS commit: src/usr.sbin/envstat
On Fri, Dec 14, 2012 at 05:51:44PM +0100, Marc Balmer wrote: Am 14.12.2012 um 17:07 schrieb Joerg Sonnenberger jo...@britannica.bec.de: On Fri, Dec 14, 2012 at 08:59:37AM +, David Laight wrote: Similarly it just isn't worth trying to free resources prior to program exit. Have you ever waited while a big C++ program runs all its destructors, paging in code and data just to exit! Depends, it makes tools like valgrind a lot easier to use. Source code should be optimized for readability, performance, stability, and not for debug tools. Ironically, proper ressource managements tends to make it much easier to fulfill at least two of the three items... Joerg
Re: CVS commit: src/usr.sbin/envstat
Am 14.12.2012 um 18:05 schrieb Joerg Sonnenberger jo...@britannica.bec.de: On Fri, Dec 14, 2012 at 05:51:44PM +0100, Marc Balmer wrote: Am 14.12.2012 um 17:07 schrieb Joerg Sonnenberger jo...@britannica.bec.de: On Fri, Dec 14, 2012 at 08:59:37AM +, David Laight wrote: Similarly it just isn't worth trying to free resources prior to program exit. Have you ever waited while a big C++ program runs all its destructors, paging in code and data just to exit! Depends, it makes tools like valgrind a lot easier to use. Source code should be optimized for readability, performance, stability, and not for debug tools. Ironically, proper ressource managements tends to make it much easier to fulfill at least two of the three items... proper resource management is a good thing - if the code continues to run. In this case, where the program exits, there no benefit from freeing up memory etc.
Re: CVS commit: src/usr.sbin/envstat
On Fri, Dec 14, 2012 at 06:18:57PM +0100, Marc Balmer wrote: Ironically, proper ressource managements tends to make it much easier to fulfill at least two of the three items... proper resource management is a good thing - if the code continues to run. In this case, where the program exits, there no benefit from freeing up memory etc. And this is the reason why I've programmed in Java. And why I like C, on the other hand. And why we have Lua in the base system. - Jukka.
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 | -
re: CVS commit: src/usr.sbin/envstat
Module Name: src Committed By: pgoyette Date: Sat Jun 4 13:29:03 UTC 2011 Modified Files: src/usr.sbin/envstat: envstat.c Log Message: Since there is no longer a value-avg property, remove the code that extracts it. what's the behaviour of old envstat and new kernel? thanks. .mrg.
re: CVS commit: src/usr.sbin/envstat
On Sun, 5 Jun 2011, matthew green wrote: Module Name:src Committed By: pgoyette Date: Sat Jun 4 13:29:03 UTC 2011 Modified Files: src/usr.sbin/envstat: envstat.c Log Message: Since there is no longer a value-avg property, remove the code that extracts it. what's the behaviour of old envstat and new kernel? Old envstat simply retrieved the value_avg from the returned prop-dict (if the value was exported by the kernel) but never used the resulting value. Furthermore, the value_avg field was exported, if and only if the ENVSYS_FVALID_AVG flag was set. Nothing ever sets the flag, and the only use of value_avg is within dev/pci/arcmsr.c where it is used for a purpose totally unrelated to sysmon (it is used to correlate the sensor to a raidset member). So ... The value was never exported, but even if it had been, it would have been ignored. I simply cleaned up to remove the code that would never get triggered. - | 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 | -
CVS commit: src/usr.sbin/envstat
Module Name:src
Committed By: pgoyette
Date: Mon Feb 15 22:37:14 UTC 2010
Modified Files:
src/usr.sbin/envstat: config.c config_lex.l envstat.c
Log Message:
Update userland envstat(8) to handle new {high,maximum}-capacity limits.
To generate a diff of this commit:
cvs rdiff -u -r1.8 -r1.9 src/usr.sbin/envstat/config.c
cvs rdiff -u -r1.6 -r1.7 src/usr.sbin/envstat/config_lex.l
cvs rdiff -u -r1.76 -r1.77 src/usr.sbin/envstat/envstat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
CVS commit: src/usr.sbin/envstat
Module Name:src Committed By: cnst Date: Fri Feb 12 05:02:40 UTC 2010 Modified Files: src/usr.sbin/envstat: envstat.c Log Message: remove the fourth (empty) column from the -T printouts; ok pgoyette To generate a diff of this commit: cvs rdiff -u -r1.74 -r1.75 src/usr.sbin/envstat/envstat.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/usr.sbin/envstat
Module Name:src Committed By: wiz Date: Sat Jan 30 08:57:49 UTC 2010 Modified Files: src/usr.sbin/envstat: envstat.8 Log Message: New sentence, new line. To generate a diff of this commit: cvs rdiff -u -r1.52 -r1.53 src/usr.sbin/envstat/envstat.8 Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/usr.sbin/envstat
Module Name:src Committed By: pgoyette Date: Sat Jan 30 02:56:39 UTC 2010 Modified Files: src/usr.sbin/envstat: envstat.8 envstat.c Log Message: Since we never have both a limit value and a limit %capacity value, remove the Capacity column. Reduce inter-column spacing, and display all four limits on one page, rather than using the -W command line option to switch between critical and warning limits. (The -W option is still permitted, but has no effect.) %capacity limits are displayed in the WarnMin and CritMin columns, but have a trailing % sign. To generate a diff of this commit: cvs rdiff -u -r1.51 -r1.52 src/usr.sbin/envstat/envstat.8 cvs rdiff -u -r1.72 -r1.73 src/usr.sbin/envstat/envstat.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
