X-Authentication-Warning: delorie.com: mail set sender to geda-user-bounces using -f X-Recipient: geda-user AT delorie DOT com Message-ID: <56E866C8.9080508@xs4all.nl> Date: Tue, 15 Mar 2016 20:47:20 +0100 From: "Bert Timmerman (bert DOT timmerman AT xs4all DOT nl) [via geda-user AT delorie DOT com]" User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.19) Gecko/20110429 Fedora/2.0.14-1.fc13 SeaMonkey/2.0.14 MIME-Version: 1.0 To: geda-user AT delorie DOT com Subject: Re: [geda-user] pcb: more memory leaks References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: geda-user AT delorie DOT com gedau AT igor2 DOT repo DOT hu wrote: > Hi all, > > after removing glib from pcb-rnd core, I am in a better position to > valgrind the code. (Bert asked me to send leaks I find so that they > can be fixed in mainline too; rNUMBERS are revision numbers in > pcb-rnd's svn for reference). > > (Note: some of these leaks are "virtual" in the sense that they are > allocated once and would be free'd at exit. Some say such allocation > should not be free'd because it's just spending/wasting some CPU > cycles on heap administration of memory that will be discarded anyway. > I do not want to generate discussion about this topic; I decided to > fix them in pcb-rnd and I list them so mainline pcb developers can > decide whether they should be fixed.) > > 1. Easy ones > > These are relatively easy to fix, as they are local. > > - InitBuffers() allocates paste buffers, but there's no > UninitBuffers() to free them (r1227) > > - on quit, either by the GUI hid's main loop returning or by the quit > action, FreePCBMemory() is called, but the actual PCB data allocation > is not freed, only its fields (r1229, r1238, depends on r1223, see later) > > - on quit, default layer names in settings are not free'd (r1231, > depends on r1223) > > - pcb parser: parse_y.y: in the "flags:" rule, after string_to_flags, > $1 should be free()'d; the .l allocates the string, the parser should > free it. > > - pcb parser: parse_y.y: a similar case in rule "pcbflags:" for $3 in > the second subrule (where flags are spcified as a string, not as an > integer) > > - pcb parser: parse_y.y: two similar case in rule "pcbstyles:" for $3 > > - pcb parser: parse_y.y: in the "layers:" rule, at the end free($5) if > $5 is not NULL (but do NOT free($4)) > > - strflags.c: alloc_buf(); it allocates and maintains a cache of 10 > buffers; at the end none of this get free()'d (r1233) > > - strflags.c: grow_layer_list(); it allocates and maintains an array > of layers; at the end none of this get free()'d (r1237) > > - main.c: pcblibdir, homedir, bindir, exec_prefix are allocated in > InitPaths() but are never free()'d (r1234) > > - main.c: program_directory is never free()'d (r1235) > > - main.c: Settings.MakeProgram and Settings.GnetlistProgram are never > free()'d (r1236) > > > 2. Hard ones > > These require new API and/or biggish refactoring. > > - there's no infrastructure for properly uninitializing GUI hids so > they have no chance to free resources they allocated for the gui (r1222) > > - the quit action just runs exit(0); instead, it should issue a call > to the GUI hid and ask it to exit cleanly (r1223) > > - resource_parse() allocates a tree for the resources loaded, but > there's no call to free() this tree. I plan to replace the current > resource code with lihata on the long run, so I did not properly fix > this one, just worked it around using the "leaky" "auto-free-at-exit" > allocation mechanism (free_atexit.[ch]) > > The list is not complete, it reflects the result of my first leak > cleanup session. There would be subsequent sessions soon. > > Regards, > > Igor2 > > Hi Igor2 and list members, I'm just made it past r1238 and I see a patch series arising in topic branch "home/bert/plugging_leaks". More commits will be added in the coming days (depending on available free cycles ;-) Some may need to get squashed before getting them into "master". Any volunteers for reviewing these with a second pair of eyes ? The plan is to cherry-pick commits into "master" after reviewing, so you don't have to wade through all of them. Kind regards, Bert Timmerman.