Log for on 10th December 2016:
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
04:32:27  *** ChanServ sets mode: +v Supercheese
08:17:17  *** Supercheese has quit IRC
12:35:08  *** adf88 has joined
12:35:08  *** ChanServ sets mode: +v adf88
12:36:07  *** frosch123 has joined
12:36:07  *** ChanServ sets mode: +v frosch123
12:40:52  <adf88> There is a memleak, when clearing the RailTypeInfo _railtypes array here:
12:41:06  <adf88> and when adding new rail type here:
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>
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> <- 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> <- 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: || 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: || 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>
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> <- 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: || 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:
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> <- 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> <- i guess that means memset
15:36:09  <Rubidium> with empty_railtype in the assignment it doesn't crash
15:42:42  <frosch123> <- i guess that is safe
15:43:52  <Rubidium> <- 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: || 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
16:10:45  *** frosch123 has quit IRC
16:30:15  *** welshdragon has joined
16:30:32  *** welshdragon has quit IRC
16:31:01  *** welshdragon has joined
16:31:26  *** welshdragon has joined
16:40:42  *** welshdragon has quit IRC
16:41:07  *** welshdragon has joined
16:52:55  *** welshdragon has quit IRC
16:55:04  *** welshdragon has joined
17:00:55  *** welshdragon has quit IRC
17:02:53  *** welshdragon has joined
17:05:16  *** welshdragon has quit IRC
17:05:37  *** welshdragon has joined
17:33:04  *** welshdragon has quit IRC
17:33:57  *** welshdragon has joined
18:45:40  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27691 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
19:18:39  *** Supercheese has joined
19:18:39  *** ChanServ sets mode: +v Supercheese
20:34:04  *** welshdragon has quit IRC
20:34:22  *** welshdragon has joined
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
22:28:38  *** welshdragon has quit IRC
22:28:51  *** welshdragon has joined

Powered by YARRSTE version: svn-trunk