Ticket #1912 (closed defect: wontfix)

Opened 10 years ago

Last modified 10 years ago

Remove unused free_codepages_list

Reported by: metux Owned by:
Priority: major Milestone:
Component: mc-core Version:
Keywords: Cc:
Blocked By: Blocking:
Branch state: Votes for changeset:

Description (last modified by slyfox) (diff)

The free_codepages_list() function is only called on termination, where it's unnecessary, since the process will terminate a few cycles later.

(in general, those things aren't necessary on multiprocessing-platforms at all)

NAK'ed-by: slyfox

Change History

comment:1 follow-up: ↓ 2 Changed 10 years ago by ossi

note that it makes sense to have cleanup functions in a conditional branch to simplify working with memory leak checkers.

comment:2 in reply to: ↑ 1 ; follow-ups: ↓ 3 ↓ 4 Changed 10 years ago by metux

Replying to ossi:

note that it makes sense to have cleanup functions in a conditional branch to simplify working with memory leak checkers.

hmm, very good point, i hadnt considered.
perhaps we should add some --enable-memory-cleanup option which
causes this code to be enabled (otherwise it's just empty macros or inlines).

a perhaps more sophisticated approach might be using an garbage collection.

comment:3 in reply to: ↑ 2 Changed 10 years ago by andrew_b

  • Status changed from new to closed
  • Version version not selected deleted
  • Resolution set to wontfix
  • Milestone 4.7 deleted

Replying to metux:

hmm, very good point, i hadnt considered.
perhaps we should add some --enable-memory-cleanup option which
causes this code to be enabled (otherwise it's just empty macros or inlines).

a perhaps more sophisticated approach might be using an garbage collection.

Absolutely no reason to introduce some strange options. All memory allocated in program must be freed in that program. Just use this simple rule and don't confuse other people.

comment:4 in reply to: ↑ 2 ; follow-up: ↓ 6 Changed 10 years ago by ossi

Replying to metux:

perhaps we should add some --enable-memory-cleanup option which
causes this code to be enabled (otherwise it's just empty macros or inlines).

typically this is a run-time option, enabled via an environment variable (often, glibc's MALLOC_CHECK_ is "borrowed"). also, valgrind-dev provides a header file with a function to check whether the program is run under valgrind control.

a perhaps more sophisticated approach might be using an garbage collection.

bit of overkill for mc, huh? ;)

Replying to andrew_b:

All memory allocated in program must be freed in that program.

which the OS will happily do for us without even being asked ...
programs which do a lot of allocations (probably not mc) will be considerably slowed down on exit if they free everything explicitly.
on top of that, some allocators will cause basically all of the virtual memory to be swapped back into ram just for being able to free it, which will slow down the exit even more.

Just use this simple rule and don't confuse other people.

yes, life is so much simpler if one defines arbitrary rules and ignores the implications. ;)

comment:5 Changed 10 years ago by slyfox

  • Description modified (diff)

Correctness matters.

  • mc is not the program, expected to be ran 10K times in a minute
  • it will take additional cryptic unneeded code for leak checkers. "code for tool" is not nice. I like "tool for code" more :]
  • If some handicap system wants it - devs can "patch" code themselves (they anyways will do that heavily)

comment:6 in reply to: ↑ 4 Changed 10 years ago by metux

typically this is a run-time option, enabled via an environment variable (often, glibc's MALLOC_CHECK_ is "borrowed"). also, valgrind-dev provides a header file with a function to check whether the program is run under valgrind control.

Yes, but if we do the free's on exit just to make valgrind working
(so it doesnt show up lots of false-positives), this only has to
be done in an debug build.

a perhaps more sophisticated approach might be using an garbage collection.

bit of overkill for mc, huh? ;)

No, I dont think so. With GC we simply dont have to care about
deallocation anymore. In general that's a big area of possible bugs,
which gets eliminated by an GC.

I'm not sure about the side effects (eg. free'ing OS resources
like fd's the right time).

Replying to andrew_b:

All memory allocated in program must be freed in that program.

which the OS will happily do for us without even being asked ...
programs which do a lot of allocations (probably not mc) will be considerably slowed down on exit if they free everything explicitly.
on top of that, some allocators will cause basically all of the virtual memory to be swapped back into ram just for being able to free it, which will slow down the exit even more.

ACK. Besides the valgrind issue, free's on exit is totally unnecessary. And valgrind itself serves just to catch issues
which are elimited by an GC.

Note: See TracTickets for help on using tickets.