Alexander Bluhm <alexander.bl...@gmx.net> writes:
> On Fri, Jul 14, 2017 at 10:15:49PM +0200, Jeremie Courreges-Anglas wrote:
>> I find this more readable.
>
> yes, it is
>
>> /* Step 4, compress the log.0 file if configured to do so and free */
>> - while (p) {
>> + TAILQ_FOREACH_SAFE(p, &runlist, next, tmp) {
>> if ((p->flags & CE_COMPACT) && (p->flags & CE_ROTATED) &&
>> p->numlogs > 0)
>> compress_log(p);
>> - q = p;
>> - p = p->next;
>> - free(q);
>> + free(p);
>> }
>
> The usual idiom for freeing a list is:
> while ((p = TAILQ_FIRST(&runlist))) {
> TAILQ_REMOVE(&runlist, p, next);
> ...
> free(p);
> }
>
> Your code leaves a list of freed objects which is not clean.
>
> As we are close to exit, you could also not free anything and use
> a TAILQ_FOREACH.
I had a diff to (hopefully) free all the allocated memory. But I guess
that the total amount allocated doesn't matter that much and simpler may
be better. ok?
Index: newsyslog.c
===================================================================
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
retrieving revision 1.103
diff -u -p -r1.103 newsyslog.c
--- newsyslog.c 14 Jul 2017 20:51:17 -0000 1.103
+++ newsyslog.c 14 Jul 2017 21:11:46 -0000
@@ -263,11 +263,10 @@ main(int argc, char **argv)
sleep(5);
/* Step 4, compress the log.0 file if configured to do so and free */
- TAILQ_FOREACH_SAFE(p, &runlist, next, tmp) {
+ TAILQ_FOREACH(p, &runlist, next) {
if ((p->flags & CE_COMPACT) && (p->flags & CE_ROTATED) &&
p->numlogs > 0)
compress_log(p);
- free(p);
}
/* Wait for children to finish, then exit */
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
