Problem 1020

Summary:	G4OpBoundaryProcess::G4Swap(G4Material, G4Material) has no effect
Product:	Geant4	Reporter:	Stan Seibert <volsung>
Component:	processes/optical	Assignee:	gum
Status:	RESOLVED FIXED
Severity:	trivial	CC:	Gabriele.Cosmo, gum, Gunter.Folger
Priority:	P5
Version:	9.0
Hardware:	All
OS:	All

Description Stan Seibert 2008-08-12 20:57:01 CEST

G4OpBoundaryProcess::G4Swap(G4Material*, G4Material*) was intended to swap to material pointers, much like G4Swap(G4double* a, G4double* b) does for doubles.  However, the parameters need to be pointers to pointers in order to have any effect.  The current G4Swap(Material*,Material*) does nothing.

Reading through G4OpBoundary::DielectricDielectric(), where this version of G4Swap is used, I find that this bug has no actual effect on the code behavior.  Material1 and Material2 are never accessed, beyond the attempted swapping.  Rindex1 and Rindex2 are swapped as expected, since they are doubles.

G4Swap(Material*,Material*) should be changed to G4Swap(Material**, Material**) to avoid confusion in the future.

Comment 1 gum 2008-08-15 02:29:12 CEST

Thanks Stan,

both for pointing out the C++ error and for explaining the problem! 

Still, it took me a while to graps why the method as written does not work. To summarize and to say it in my words, the gist of your bug report comes down to the fact that in C++, to have any effect in the calling program, arguments to methods must be passed by reference. If the arguments are not passed by reference, copies are made from the arguments, and the code in the method only affects those copies; the original objects (pointers) in the calling code are unaffected.

Now it turns out, that in the next G4 release, G4OpBoundaryProcess::G4Swap will have been replaced by the global utility:

http://www-geant4.kek.jp/lxr/source/global/management/include/templates.hh#L114

which, believe it or not, is in ERROR all the same!!!

I shall fix this with:

template <class T>
inline void G4SwapPtr(T** a, T** b) {
  T* tmp=*a;
  *a = *b;
  *b = tmp;
}

You are right, the bug did no harm in G4OpBoundaryProcess and, in fact as you pointed out, the swap of the material pointers is not needed in the code. Still, I'll keep it in for readability of the code.

Cheers, Peter