Times are UTC Toggle Colours
00:14:03 *** welshdragon has quit IRC 04:14:24 *** Supercheese has quit IRC 04:32:27 *** Supercheese has joined #openttd.dev 04:32:27 *** ChanServ sets mode: +v Supercheese 08:17:17 *** Supercheese has quit IRC 12:35:08 *** adf88 has joined #openttd.dev 12:35:08 *** ChanServ sets mode: +v adf88 12:36:07 *** frosch123 has joined #openttd.dev 12:36:07 *** ChanServ sets mode: +v frosch123 12:40:52 <adf88> There is a memleak, when clearing the RailTypeInfo _railtypes array here: http://hg.openttd.org/trunk.hg/file/95909be228c2/src/rail_cmd.cpp#l69 12:41:06 <adf88> and when adding new rail type here: http://hg.openttd.org/trunk.hg/file/95909be228c2/src/rail_cmd.cpp#l154 12:41:23 <adf88> RailTypeInfo::alternate_labels, which is a SmallVector, will not be freed properly 12:43:17 <adf88> and I have a fix :) 12:43:18 <adf88> http://pastebin.com/BzRFsS22 12:49:57 <adf88> mixing C and C++ was the source of the issue so I changed it to pure C++ 12:52:33 <adf88> and if you wander, there is no "placement delete" elsewhere in the code or anything else that would clear the vector 12:57:59 <frosch123> interesting, more road stuff :) 13:01:19 <frosch123> it needs a default constructor for RailtypeInfo though 13:01:29 <frosch123> various compilers do not zero-initialise POD members 13:01:32 <adf88> implicit will do 13:02:19 <adf88> RailTypeInfo() 13:02:27 <adf88> will zero-initialize it 13:02:43 <adf88> these "()" 13:03:01 <adf88> guarantee that 13:03:19 <frosch123> according to my experience that does not propagate to POD members 13:04:07 <adf88> but on a POD itself it does 13:05:05 <adf88> there are two kind of implicitly generated constructors - trivial and zero-init 13:05:19 <adf88> the zero-init constructor propagates on members 13:05:33 <adf88> AFAIR 13:08:10 <frosch123> http://stackoverflow.com/questions/4932781/why-is-a-pod-in-a-struct-zero-initialized-by-an-implicit-constructor-when-creati <- ok, that seems to agree with you :) 13:08:15 <adf88> e.g. 13:08:25 <adf88> "int()" makes a 0 13:08:28 <adf88> and 13:08:51 <adf88> "struct Foo { int i; }; Foo();" 13:08:55 <adf88> makes {0 } 13:13:01 <frosch123> it needs a rti->alternate_labels.Clear(), right? 13:13:18 <adf88> no 13:13:35 <adf88> this will happen atomatically on destruction 13:13:44 <adf88> untill we dont touch 13:13:46 <frosch123> that's not what i mean 13:13:48 <adf88> memset 13:13:55 <adf88> everything is ok 13:13:59 <frosch123> in AllocateRailType it copies RAILTYPE_RAIL 13:14:17 <frosch123> i am not sure whether alternate_labels is garanteed to be empty at that point 13:14:22 <adf88> vectros do copy themseves properly 13:14:43 <frosch123> yes, but it is not supposed to be copied 13:15:00 <adf88> ? 13:15:06 <adf88> why not? 13:15:25 <frosch123> but copying _railtypes[RAILTYPE_RAIL] is weird in itself, i wonder whether it shoudl copy _original_railtypes instead 13:15:52 <frosch123> adf88: alternate_labels contains exclusive entries 13:16:03 <frosch123> you cannot make two different railtypes alternative to a third one 13:16:16 <adf88> vectors in _original_railtypes are all empty 13:16:43 <adf88> oh, i see 13:16:47 <frosch123> RailTypeReserveInfo is the only place that calls AllocateRailType 13:16:58 <frosch123> but it also sets alternate_labels 13:17:11 <frosch123> i guess we should just copy _original_railtypes instead 13:17:17 <frosch123> copying _railtypes is more than weird 13:17:43 <adf88> _railtypes[RAILTYPE_RAIL].alternate_railtypes 13:17:47 <adf88> will always be empty 13:17:57 <adf88> because first items in _railtype array 13:18:07 <adf88> are always the same as _original_railtype 13:18:19 <frosch123> no, grfs can modify them 13:18:24 <adf88> yes? 13:18:30 <adf88> so there is another bug :) 13:18:44 <frosch123> it's not another 13:19:01 <adf88> oh, sorry 13:19:06 <frosch123> the only thing they can modify before AllocateRailType is called, is just alternate_railtypes 13:19:11 <adf88> it's the one that I did 13:19:47 <adf88> I was thinking before, why not _orignal_railtypes 13:19:51 <adf88> is used 13:20:02 <adf88> when creating a new raitypeinfo 13:20:08 <frosch123> https://paste.openttdcoop.org/pgau7ddof?/pgau7ddof <- let's make it overly explicit 13:20:14 <frosch123> copy original and clear alternative labels :) 13:20:26 <adf88> original is emty 13:20:50 <adf88> but there may be a reason why _railtype is used and not _original_railtypes 13:23:36 <frosch123> the "using original stuff as defauilt" behaviour comes from houses and industries 13:23:51 <frosch123> but also they copy the original data, and not a possibly already modified data 13:23:59 <frosch123> because that would be quite surprisnig for newgrf 13:24:15 <adf88> yes I agree 13:24:19 <adf88> the would be 13:24:29 <frosch123> but, i'll make it two commits, to make it explicit :) 13:24:53 <adf88> why the changes made to the railtype should propagate? 13:25:07 <adf88> anyway 13:25:18 <adf88> cleaing the list is not mandatory 13:25:36 <adf88> _original_railtypes are alsways clear 13:25:55 <adf88> "rti->alternate_labels.Clear();" - unnecesary 13:26:32 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27687 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:27:15 <adf88> so there was another bug :) 13:28:48 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27688 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:29:15 <adf88> anyway, I thought that not using __original_railtype is intended and copying labels was the right behaviour 13:30:10 <frosch123> there is probably the same bug in nrt :) 13:32:32 <adf88> whre? 13:32:59 <frosch123> https://github.com/andythenorth/NotRoadTypes/commits/road-and-tram-types 13:33:06 <frosch123> i am fixing it just now there as well 13:33:20 <adf88> :) 14:13:01 <frosch123> hmm, windows compilation failed 14:14:49 <frosch123> https://farm.openttd.org/browse/OTTD-TEST-W64BIT-4364/log <- no idea why 14:15:05 <frosch123> it couldn't possibly be less specific :p 14:33:59 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27689 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 15:11:39 <Rubidium> at least it's reproducable 15:13:08 <Rubidium> at work the build environment freezes for no apparant reason. Kill the builder and rebuild... build succesful, but no clue why 15:13:28 <frosch123> :) 15:20:03 <Rubidium> frosch123: https://paste.openttdcoop.org/p7y9ziyia 15:21:11 <frosch123> ah, so most likely all the zero-initialisation which we talked earlier about was not ture 15:21:20 <frosch123> well, we could just check that 15:22:33 <Rubidium> I'm busy on the farm's builder instance to try to put a breakpoint at a useful location 15:23:43 <Rubidium> what should I look at exactly? 15:24:32 <frosch123> https://paste.openttdcoop.org/pasxaysto?/pasxaysto <- i would try something like that 15:25:22 <frosch123> Rubidium: the suspicion is that the RailtypeInfo() implicitly created constructor does not zero-initialise POD members 15:29:45 <Rubidium> empty_railtypes gets initialised properly? 15:30:11 <frosch123> i would hope that the "static" would make a difference 15:30:46 <frosch123> otherwise it may need a long constructor, or we readd the memset, which is not that horrible for SmallVector 15:32:21 <frosch123> hmm, "static const" could be better 15:33:03 <frosch123> oh, that actually does not compile 15:34:16 <Rubidium> okay, the Railtypeinfo constructor does either put really large numbers in all the string fields or just doesn't touch things compared to empty_railtype which looked almost completely zeroed 15:34:17 <frosch123> https://paste.openttdcoop.org/pkijmq3kz <- i guess that means memset 15:36:09 <Rubidium> with empty_railtype in the assignment it doesn't crash 15:42:42 <frosch123> https://paste.openttdcoop.org/pokdmekqg?/pokdmekqg <- i guess that is safe 15:43:52 <Rubidium> https://paste.openttdcoop.org/posoqdnuc <- just to show what happens 15:46:15 <frosch123> i guess it is different in newer msvc versions 15:46:31 <frosch123> i think it also changed in g++ at some point 15:47:18 <Rubidium> that at least does not crash openttd at start up 15:49:26 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27690 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 15:57:51 <Rubidium> this build looks more promising 15:58:07 <frosch123> yay, thanks for debugging :) 16:10:45 *** frosch has joined #openttd.dev 16:10:45 *** frosch123 has quit IRC 16:30:15 *** welshdragon has joined #openttd.dev 16:30:32 *** welshdragon has quit IRC 16:31:01 *** welshdragon has joined #openttd.dev 16:31:26 *** welshdragon has joined #openttd.dev 16:40:42 *** welshdragon has quit IRC 16:41:07 *** welshdragon has joined #openttd.dev 16:52:55 *** welshdragon has quit IRC 16:55:04 *** welshdragon has joined #openttd.dev 17:00:55 *** welshdragon has quit IRC 17:02:53 *** welshdragon has joined #openttd.dev 17:05:16 *** welshdragon has quit IRC 17:05:37 *** welshdragon has joined #openttd.dev 17:33:04 *** welshdragon has quit IRC 17:33:57 *** welshdragon has joined #openttd.dev 18:45:40 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27691 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 19:18:39 *** Supercheese has joined #openttd.dev 19:18:39 *** ChanServ sets mode: +v Supercheese 20:34:04 *** welshdragon has quit IRC 20:34:22 *** welshdragon has joined #openttd.dev 20:34:44 *** frosch has quit IRC 21:09:10 *** adf88 has quit IRC 22:20:43 *** welshdragon has quit IRC 22:20:58 *** welshdragon has joined #openttd.dev 22:28:38 *** welshdragon has quit IRC 22:28:51 *** welshdragon has joined #openttd.dev