[CMSSW_7_0_X] Fix maybe-uninitialized errors in RecoEcal/EgammaClusterAlgos

[davidlt]

Fix the following two errors on slc6_amd64_gcc481:

RecoEcal/EgammaClusterAlgos/src/PFECALSuperClusterAlgo.cc:276:15: error:
'IsClusteredWithSeed.{anonymous}::IsClustered::phiwidthSuperCluster_'
may be used uninitialized in this function [-Werror=maybe-uninitialized]
   IsClustered IsClusteredWithSeed(seed,_clustype,_useDynamicDPhi);

RecoEcal/EgammaClusterAlgos/src/PFECALSuperClusterAlgo.cc:276:15: error:
'IsClusteredWithSeed.{anonymous}::IsClustered::etawidthSuperCluster_'
may be used uninitialized in this function [-Werror=maybe-uninitialized]

In PFECALSuperClusterAlgo::buildSuperCluster we only check for
PFLayer::ECAL_BARREL and PFLayer::ECAL_ENDCAP. If any other
PFLayer::Layer is returned by seed->the_ptr()->layer()
phiwidthSuperCluster_ and etawidthSuperCluster_ are uninitialized.

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

[cmsbuild]

A new Pull Request was created by @davidlt for CMSSW_7_0_X.

[CMSSW_7_0_X] Fix maybe-uninitialized errors in RecoEcal/EgammaClusterAlgos

It involves the following packages:

RecoEcal/EgammaClusterAlgos

@thspeer, @slava77 can you please review it and eventually sign? Thanks.

[slava77]

working @slava77

[slava77]

tried to cms-merge-topic --unsafe 587
in CMSSW_7_0_X_2013-08-19-0200
got 140 files changed, 2145 insertions(+), 2042 deletions(-)
why is it so difficult for such a basic change?
sigh
@ktf @davidlt

[ktf]

Because git-cms-merge-topic brings in the tip of the branch, so that we are sure you are testing together with the latest code have in the repository.

[slava77]

yeah, I get this, but I think that for trivial changes the tracking should be off,
or, more operationally clearly, these trivial changes have to be made based on an older release.
If I need to fix a scratch on a car, I don't always want to have a better engine forced on me to be installed.

@davidlt please let me know which IB was the base of your topic.

[davidlt]

@slava77 it's CMSSW_7_0_X_2013-08-20-1400

[slava77]

Thanks, it's clean here.
Still, I'd really prefer that all trivial fixes are made on top of the last pre-release (or even last major release).
It's clear here that the incremental diff in performance is irrelevant,
but for something more useful it better be easily mergeable

[ktf]

Well, this is just the behaviour of cms-merge-topic, which we can change.

You could have picked up the changes by hand by fetching the branch and then doing:

git fetch https://github.com/davidlt/cmssw fix-reco-maybe-uninitialized
git cms-sparse-checkout fix-reco-maybe-uninitialized $CMSSW_VERSION
# The above could have been done by hand, since it would have been enough you had Reco* in your .git/info/sparse-checkout
git merge fix-reco-maybe-uninitialized

which would have had the behaviour you wanted (not tested, but that's the idea). We can change the default behaviour of cms-merge-topic (I'll do that and add an option -A or something for the current behaviour), of course, but this is in the same league of checking something without doing checkdeps, IMHO.

[nclopezo]

Hi,

I tested the changes on CMSSW_7_0_X_2013-08-22-0200, all tests passed.

You can see the logs here:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc472/209/console

[slava77]

On 8/22/13 2:09 PM, Giulio Eulisse wrote:

Well, this is just the behaviour of |cms-merge-topic|, which we can change.

You could have picked up the changes by hand by fetching the branch and
then doing:

|git fetch https://github.com/davidlt/cmssw fix-reco-maybe-uninitialized
git cms-sparse-checkout fix-reco-maybe-uninitialized $CMSSW_VERSION

The above could have been done by hand, since it would have been enough you had Reco* in your .git/info/sparse-checkout

git merge fix-reco-maybe-uninitialized
|

which would have had the behaviour you wanted. We can change the default
behaviour of |cms-merge-topic| (I'll do that and add an option -A or
something for the current behaviour), of course, but this is in the same
league of checking something without doing checkdeps, IMHO.

I'm not really arguing that the merge-topic behavior need to be changed.

We have to remember our realities:

• we do not rebuild the full repo when we do the tests
• only pre-releases live reasonably long
• major/minor releases live for a rather long time and that's where people
  (not bleeding edge developers) really work
• we often need to apply simple fixes to the past versions

With the way git works, if you put the topic/feature on a topic branch
based early enough
in the release branch, your topic/feature can be easily applied
everywhere later.
A mode of operation "development should be done in the head" is not a
good choice.

Cheers

    --slava

—
Reply to this email directly or view it on GitHub
#587 (comment).

---

Vyacheslav (Slava) Krutelyov
TAMU: Physics Dept Texas A&M MS4242, College Station, TX 77843-4242
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)

---

[slava77]

signing b0ec64d

[cmsbuild]

The following categories have been signed by @slava77: Reconstruction

@cms-git-reconstruction

[slava77]

basic matrix tests ok, no differences as expected
(run in CMSSW_7_0_X_2013-08-20-1400)