{"action":"created","issue":{"url":"https://api.github.com/repos/cms-sw/cmssw/issues/24146","repository_url":"https://api.github.com/repos/cms-sw/cmssw","labels_url":"https://api.github.com/repos/cms-sw/cmssw/issues/24146/labels{/name}","comments_url":"https://api.github.com/repos/cms-sw/cmssw/issues/24146/comments","events_url":"https://api.github.com/repos/cms-sw/cmssw/issues/24146/events","html_url":"https://github.com/cms-sw/cmssw/pull/24146","id":346241572,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA1MTUzODc0","number":24146,"title":"Most static analyzer memory errors fixed","user":{"login":"guitargeek","id":6578603,"node_id":"MDQ6VXNlcjY1Nzg2MDM=","avatar_url":"https://avatars2.githubusercontent.com/u/6578603?v=4","gravatar_id":"","url":"https://api.github.com/users/guitargeek","html_url":"https://github.com/guitargeek","followers_url":"https://api.github.com/users/guitargeek/followers","following_url":"https://api.github.com/users/guitargeek/following{/other_user}","gists_url":"https://api.github.com/users/guitargeek/gists{/gist_id}","starred_url":"https://api.github.com/users/guitargeek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/guitargeek/subscriptions","organizations_url":"https://api.github.com/users/guitargeek/orgs","repos_url":"https://api.github.com/users/guitargeek/repos","events_url":"https://api.github.com/users/guitargeek/events{/privacy}","received_events_url":"https://api.github.com/users/guitargeek/received_events","type":"User","site_admin":false},"labels":[{"id":52881463,"node_id":"MDU6TGFiZWw1Mjg4MTQ2Mw==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/alca-pending","name":"alca-pending","color":"fbca04","default":false},{"id":52881179,"node_id":"MDU6TGFiZWw1Mjg4MTE3OQ==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/analysis-pending","name":"analysis-pending","color":"fbca04","default":false},{"id":673935047,"node_id":"MDU6TGFiZWw2NzM5MzUwNDc=","url":"https://api.github.com/repos/cms-sw/cmssw/labels/code-checks-approved","name":"code-checks-approved","color":"2cbe4e","default":false},{"id":88653795,"node_id":"MDU6TGFiZWw4ODY1Mzc5NQ==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/comparison-pending","name":"comparison-pending","color":"fbca04","default":false},{"id":52881671,"node_id":"MDU6TGFiZWw1Mjg4MTY3MQ==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/db-pending","name":"db-pending","color":"fbca04","default":false},{"id":52820549,"node_id":"MDU6TGFiZWw1MjgyMDU0OQ==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/dqm-pending","name":"dqm-pending","color":"fbca04","default":false},{"id":52881696,"node_id":"MDU6TGFiZWw1Mjg4MTY5Ng==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/generators-pending","name":"generators-pending","color":"fbca04","default":false},{"id":52881721,"node_id":"MDU6TGFiZWw1Mjg4MTcyMQ==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/geometry-pending","name":"geometry-pending","color":"fbca04","default":false},{"id":56953091,"node_id":"MDU6TGFiZWw1Njk1MzA5MQ==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/orp-pending","name":"orp-pending","color":"fbca04","default":false},{"id":53167770,"node_id":"MDU6TGFiZWw1MzE2Nzc3MA==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/pending-signatures","name":"pending-signatures","color":"ff8000","default":false},{"id":52820010,"node_id":"MDU6TGFiZWw1MjgyMDAxMA==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/reconstruction-pending","name":"reconstruction-pending","color":"fbca04","default":false},{"id":56131152,"node_id":"MDU6TGFiZWw1NjEzMTE1Mg==","url":"https://api.github.com/repos/cms-sw/cmssw/labels/tests-pending","name":"tests-pending","color":"fbca04","default":false},{"id":530655448,"node_id":"MDU6TGFiZWw1MzA2NTU0NDg=","url":"https://api.github.com/repos/cms-sw/cmssw/labels/upgrade-pending","name":"upgrade-pending","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/cms-sw/cmssw/milestones/77","html_url":"https://github.com/cms-sw/cmssw/milestone/77","labels_url":"https://api.github.com/repos/cms-sw/cmssw/milestones/77/labels","id":3502981,"node_id":"MDk6TWlsZXN0b25lMzUwMjk4MQ==","number":77,"title":"CMSSW_10_3_X","description":null,"creator":{"login":"smuzaffar","id":4115138,"node_id":"MDQ6VXNlcjQxMTUxMzg=","avatar_url":"https://avatars3.githubusercontent.com/u/4115138?v=4","gravatar_id":"","url":"https://api.github.com/users/smuzaffar","html_url":"https://github.com/smuzaffar","followers_url":"https://api.github.com/users/smuzaffar/followers","following_url":"https://api.github.com/users/smuzaffar/following{/other_user}","gists_url":"https://api.github.com/users/smuzaffar/gists{/gist_id}","starred_url":"https://api.github.com/users/smuzaffar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/smuzaffar/subscriptions","organizations_url":"https://api.github.com/users/smuzaffar/orgs","repos_url":"https://api.github.com/users/smuzaffar/repos","events_url":"https://api.github.com/users/smuzaffar/events{/privacy}","received_events_url":"https://api.github.com/users/smuzaffar/received_events","type":"User","site_admin":false},"open_issues":44,"closed_issues":282,"state":"open","created_at":"2018-07-17T14:14:12Z","updated_at":"2018-07-31T15:32:28Z","due_on":null,"closed_at":null},"comments":6,"created_at":"2018-07-31T15:32:12Z","updated_at":"2018-07-31T15:54:06Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/cms-sw/cmssw/pulls/24146","html_url":"https://github.com/cms-sw/cmssw/pull/24146","diff_url":"https://github.com/cms-sw/cmssw/pull/24146.diff","patch_url":"https://github.com/cms-sw/cmssw/pull/24146.patch"},"body":"Since I learned so much from the friendly CMSSW community on my last pull request fixing the RecoEgamma code quality issue, I now decided to attack the memory errors pointed out by the static analyzer. I hope to cause not too much trouble. The addressed bug types were:\r\n\r\n* Bad deallocator\r\n* Double delete\r\n* Memory leak\r\n* Offset free\r\n* Use of zero allocated\r\n\r\nThis corresponds to all memory error bug types, except for Use-after-free. These can be very twisted so I didn't tackle them for the moment.\r\n\r\nSome bugs were trivial, but I elaborate my reasoning on the two more intricate kind of problems:\r\n\r\n__Memory allocation in function arguments__\r\n\r\nThis appeared quite a lot with functions that have basically the signature `template <class T> someFunction(T* object)` and use `object` only to infer the template class. They were used like `someFunction(new Object)`. I thought it's better to use the signature `template <class T> someFunction();`, and then call the function as `someFunction<Object>()` with the template class given explicitly.\r\n\r\n__Custom reference counter implementations__\r\n\r\nThis appeared twice.\r\n\r\n* First incident here:\r\n  https://github.com/cms-sw/cmssw/blob/master/TopQuarkAnalysis/TopHitFit/src/Fit_Results.cc#L98\r\n  The static analyzer doesn't understand that the object will delete itself because it inherits from `TopQuarkAnalysis/TopHitFit/interface/Refcount.h`. Since the use of this Refcount class was very rare, I just deleted it and used `std::shared_ptr` instead, which is afaik implemented as a kind of reference counter. If you think that wasn't a particularly good idea, I'm curious to learn about the use about these custom reference counter implementations!\r\n\r\n* Second incident here:\r\n  https://github.com/cms-sw/cmssw/blob/master/Calibration/Tools/src/DetIdAssociator.cc#L35\r\n  The reference counter is implemented here:\r\n  https://github.com/cms-sw/cmssw/blob/master/DataFormats/GeometrySurface/interface/ReferenceCounted.h\r\n  This custom reference counter is deeply rooted in CMSSW, so I guess it must have some advantage over `std::shared_ptr`. Therefore I didn't dare to touch it and just turned the pointers into unique pointers. This way the static analyzer will stop complaining and I hope it doesn't somehow conflict with the objects being reference counted. However, an expert advice would be very appreciated, and maybe some background on why `ReferenceCounted` was introduced and what makes it different from a shared pointer.\r\n\r\nIn the more trivial memory leaks I proceeded as follows:\r\n* Allocated memory only used in local scope => wrap in shared pointer.\r\n* Otherwise (e.g. because ownership is transferred) => use `delete` at the places where the memory was leaking. That's 90 % of the cases."},"comment":{"url":"https://api.github.com/repos/cms-sw/cmssw/issues/comments/409271899","html_url":"https://github.com/cms-sw/cmssw/pull/24146#issuecomment-409271899","issue_url":"https://api.github.com/repos/cms-sw/cmssw/issues/24146","id":409271899,"node_id":"MDEyOklzc3VlQ29tbWVudDQwOTI3MTg5OQ==","user":{"login":"cmsbuild","id":4170616,"node_id":"MDQ6VXNlcjQxNzA2MTY=","avatar_url":"https://avatars1.githubusercontent.com/u/4170616?v=4","gravatar_id":"","url":"https://api.github.com/users/cmsbuild","html_url":"https://github.com/cmsbuild","followers_url":"https://api.github.com/users/cmsbuild/followers","following_url":"https://api.github.com/users/cmsbuild/following{/other_user}","gists_url":"https://api.github.com/users/cmsbuild/gists{/gist_id}","starred_url":"https://api.github.com/users/cmsbuild/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cmsbuild/subscriptions","organizations_url":"https://api.github.com/users/cmsbuild/orgs","repos_url":"https://api.github.com/users/cmsbuild/repos","events_url":"https://api.github.com/users/cmsbuild/events{/privacy}","received_events_url":"https://api.github.com/users/cmsbuild/received_events","type":"User","site_admin":false},"created_at":"2018-07-31T15:54:06Z","updated_at":"2018-07-31T15:54:06Z","author_association":"CONTRIBUTOR","body":"The tests are being triggered in jenkins."}}
{"actor":{"display_login":"cmsbuild"}}