Skip to content

Commit 3ef42bf

Browse files
c-dilkstongtongcao
authored andcommitted
fix: several bug-prone behaviors identified by higher spotbugs ranks (#871)
* test: increase spotbugs rank let's see how high we can increase the rank without too much pain and suffering... * fix: 5 * fix: 6 * fix: thread-safe date formatter * fix: repeated conditional * fix: disable bug-prone `ClasUtilsFile.getFileList(String,String,String)`, since unused * fix: Non-virtual method call in constructor passes null for... ...non-null parameter of another constructor (`NP_NULL_PARAM_DEREF_NONVIRTUAL`) * fix: `setMass` `mass` parameter is dead upon entry but overwritten... spotbugs rank-7 bug `IP_PARAMETER_IS_DEAD_BUT_OVERWRITTEN` * fix: undeclared dependency * fix: fix or suppress some rank-8 bugs * fix: synchronized singleton getters * fix: suppress searlizable singletons * fix: remove public ctors from `SwapManager` singleton * fix: suppress static method hiding of `DetectorRepsonse.readHipoEvent` better than breaking the API... * fix: go back to rank 7 * fix: remove commented out code * fix: actuallly back to rank 7
1 parent dd53edb commit 3ef42bf

File tree

20 files changed

+139
-148
lines changed

20 files changed

+139
-148
lines changed

build-coatjava.sh

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,8 @@ fi
280280

281281
# run spotbugs
282282
if $runSpotBugs; then
283-
# mvn com.github.spotbugs:spotbugs-maven-plugin:spotbugs # spotbugs goal produces a report target/spotbugsXml.xml for each module
284-
$mvn com.github.spotbugs:spotbugs-maven-plugin:check # check goal produces a report and produces build failed if bugs
285-
# the spotbugsXml.xml file is easiest read in a web browser
286-
# see http://spotbugs.readthedocs.io/en/latest/maven.html and https://spotbugs.github.io/spotbugs-maven-plugin/index.html for more info
287-
if [ $? != 0 ] ; then echo "spotbugs failure" >&2 ; exit 1 ; fi
283+
libexec/spotbugs.sh ${mvnArgs[@]:-} || (echo "ERROR: spotbugs failure" >&2 && exit 1)
284+
echo "spotbugs spotted no bugs!"
288285
fi
289286

290287

common-tools/clas-detector/src/main/java/org/jlab/detector/swaps/SwapManager.java

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ public final class SwapManager {
5555
private ConstantsManager currConman = null;
5656
private SchemaFactory schema = null;
5757

58-
private static SwapManager instance = null;
59-
6058
public Set<String> getDetectors() {
6159
return this.detsToBanks.keySet();
6260
}
@@ -67,32 +65,7 @@ public List<String> getBanks(String detectorName) {
6765
return this.detsToBanks.get(detectorName);
6866
}
6967

70-
private SwapManager() {}
71-
72-
public static SwapManager getInstance() {
73-
if (instance == null) {
74-
instance = new SwapManager();
75-
}
76-
return instance;
77-
}
78-
79-
/**
80-
* @param detectorNames
81-
* @param prevTimestamp in CCDB format: MM/DD/YYYY
82-
* @param currTimestamp in CCDB format: MM/DD/YYYY
83-
*/
84-
public SwapManager(List<String> detectorNames, String prevTimestamp,String currTimestamp) {
85-
this.initialize(detectorNames, prevTimestamp, currTimestamp);
86-
}
87-
88-
/**
89-
* @param detectorNames
90-
* @param previous timestamp/variation used for translation tables during decoding
91-
* @param current timestamp/variation with correct translation tables
92-
*/
93-
public SwapManager(List<String> detectorNames,ConstantsManager previous,ConstantsManager current) {
94-
this.initialize(detectorNames, previous, current);
95-
}
68+
public SwapManager() {}
9669

9770
/**
9871
* @param detectorNames
@@ -265,14 +238,14 @@ private void initDetectors(List<String> detectorNames) {
265238
}
266239

267240
public static void main(String[] args) {
268-
269-
SwapManager man1 = new SwapManager(Arrays.asList("DC"),"08/10/2020","10/13/2024");
270-
System.err.println(man1.banksToTables.get("DC::tot"));
271241

272-
SwapManager noman = getInstance();
273-
System.out.println(Arrays.toString(noman.get(11014, "/daq/tt/bmt",3,5,320,0)));
274-
275-
SwapManager man = new SwapManager(Arrays.asList("BMT"),"08/10/2020","10/13/2020");
242+
SwapManager man = new SwapManager();
243+
System.out.println(Arrays.toString(man.get(11014, "/daq/tt/bmt",3,5,320,0)));
244+
245+
man.initialize(Arrays.asList("DC"),"08/10/2020","10/13/2024");
246+
System.err.println(man.banksToTables.get("DC::tot"));
247+
248+
man.initialize(Arrays.asList("BMT"),"08/10/2020","10/13/2020");
276249
man.get(11014,man.getTable("BMT"),"sector",3,6,8,0);
277250
System.out.println("SwapManager:\n"+man);
278251
System.out.println(man.get(11014,man.getTable("BMT"),"sector",99,22,33,44));

common-tools/clas-geometry/src/main/java/org/jlab/detector/geant4/DCGeant4Factory.java

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,7 @@
1-
/*
2-
* To change this license header, choose License Headers in Project Properties.
3-
* To change this template file, choose Tools | Templates
4-
* and open the template in the editor.
5-
*/
61
package org.jlab.detector.geant4;
72

8-
import java.util.ArrayList;
9-
import java.util.List;
103
import java.util.HashMap;
114
import org.jlab.geom.geant.Geant4Basic;
12-
import org.jlab.geom.prim.Line3D;
13-
import org.jlab.geom.prim.Plane3D;
14-
import org.jlab.geom.prim.Point3D;
155
import org.jlab.geom.base.ConstantProvider;
166
import org.jlab.geom.prim.Vector3D;
177

@@ -45,15 +35,11 @@ final class DCdatabase {
4535
private int nguardwires;
4636

4737
private final String dcdbpath = "/geometry/dc/";
48-
private static DCdatabase instance = null;
38+
private static DCdatabase instance = new DCdatabase();
4939

50-
private DCdatabase() {
51-
}
40+
private DCdatabase() {}
5241

5342
public static DCdatabase getInstance() {
54-
if (instance == null) {
55-
instance = new DCdatabase();
56-
}
5743
return instance;
5844
}
5945

@@ -278,7 +264,7 @@ public class DCGeant4Factory {
278264

279265
DCdatabase dbref = DCdatabase.getInstance();
280266
private Geant4Basic motherVolume = new Geant4Basic("root", "Box", 0);
281-
private HashMap<String, String> properties = new HashMap<String, String>();
267+
private HashMap<String, String> properties = new HashMap<>();
282268

283269
private int nsgwires;
284270

common-tools/clas-utils/src/main/java/org/jlab/utils/benchmark/Benchmark.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ public class Benchmark {
1616
private static final Benchmark benchmarkInstance = new Benchmark();
1717
private final Map<String,BenchmarkTimer> timerStore = new LinkedHashMap<>();
1818
private Timer updateTimer = null;
19-
20-
public Benchmark(){}
19+
20+
private Benchmark() {}
2121

2222
public static Benchmark getInstance(){
2323
return benchmarkInstance;

common-tools/clas-utils/src/main/java/org/jlab/utils/system/ClasUtilsFile.java

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -132,25 +132,7 @@ public static List<String> getFileList(String env, String rpath){
132132
}
133133
return ClasUtilsFile.getFileList(directory);
134134
}
135-
/**
136-
* returns a file list that contains files with given extension
137-
* @param env
138-
* @param rpath
139-
* @param ext
140-
* @return
141-
*/
142-
public static List<String> getFileList(String env, String rpath, String ext){
143-
String directory = ClasUtilsFile.getResourceDir(env, rpath);
144-
if(directory!=null) return new ArrayList<>();
145-
146-
List<String> files = ClasUtilsFile.getFileList(directory);
147-
List<String> selected = new ArrayList<>();
148-
for(String item : files){
149-
if(item.endsWith(ext)==true) selected.add(item);
150-
}
151-
return selected;
152-
}
153-
135+
154136
public static void writeFile(String filename, List<String> lines){
155137
System.out.println("writing file ---> " + filename);
156138
BufferedWriter writer = null;

common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77
import java.io.FileNotFoundException;
88
import java.io.IOException;
99
import java.io.PrintStream;
10-
import java.text.SimpleDateFormat;
10+
import java.time.format.DateTimeFormatter;
11+
import java.time.ZoneId;
12+
import java.time.Instant;
1113
import java.util.ArrayList;
1214
import java.util.StringTokenizer;
13-
import java.util.TimeZone;
1415
import java.util.logging.Level;
1516
import java.util.logging.Logger;
1617

@@ -40,14 +41,9 @@ public class MagneticFields {
4041
/**
4142
* A formatter to get the time in down to seconds (no day info).
4243
*/
43-
private static SimpleDateFormat formatterlong;
44-
45-
static {
46-
TimeZone tz = TimeZone.getDefault();
47-
48-
formatterlong = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss");
49-
formatterlong.setTimeZone(tz);
50-
}
44+
private static final DateTimeFormatter formatterlong = DateTimeFormatter
45+
.ofPattern("yyyy/MM/dd HH:mm:ss")
46+
.withZone(ZoneId.systemDefault());
5147

5248
// version of mag field package
5349
private static String VERSION = "1.20";
@@ -1660,7 +1656,7 @@ public String fileBaseNames() {
16601656
* @return a string representation of the current time, down to seconds.
16611657
*/
16621658
public static String dateStringLong(long longtime) {
1663-
return formatterlong.format(longtime);
1659+
return formatterlong.format(Instant.ofEpochMilli(longtime));
16641660
}
16651661

16661662
/**

common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/converter/Converter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ public void done() {
196196
zIndex++;
197197
}
198198

199-
if ((gdata[PHI].max > 31) && (gdata[PHI].max > 31)) {
199+
if (gdata[PHI].max > 31) {
200200
System.err.println("Correcting PhiMax to 360 from " + gdata[PHI].max);
201201
}
202202

@@ -850,4 +850,4 @@ public static void memoryToGEMCTorusConverter() {
850850

851851
}
852852

853-
}
853+
}

common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public class ColorScaleModel {
4141
* @param minVal the minimum value;
4242
* @param maxVal the maximum value the array of colors.
4343
*/
44-
public ColorScaleModel(double minVal, double maxVal, Color[] colors) {
44+
private ColorScaleModel(double minVal, double maxVal, Color[] colors) {
4545
_colors = colors;
4646
_minVal = minVal;
4747
_maxVal = maxVal;
@@ -52,7 +52,7 @@ public ColorScaleModel(double minVal, double maxVal, Color[] colors) {
5252
*
5353
* @return a standard blue to red color scale
5454
*/
55-
public static ColorScaleModel blueToRed() {
55+
public static synchronized ColorScaleModel blueToRed() {
5656
if (_blueToRed == null) {
5757
Color colors[] = {
5858
// new Color(0, 0, 139),
@@ -245,4 +245,4 @@ private Color getInterpretedColor(Color c1, Color c2, double fract) {
245245
return new Color(r3, g3, b3, a3);
246246
}
247247

248-
}
248+
}

common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/Environment.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public Font getCommonFont(int size) {
9595
*
9696
* @return the singleton object.
9797
*/
98-
public static Environment getInstance() {
98+
public static synchronized Environment getInstance() {
9999
if (instance == null) {
100100
instance = new Environment();
101101
}
@@ -287,4 +287,4 @@ public static void memoryReport(String message) {
287287
public static void main(String arg[]) {
288288
System.out.println(Environment.getInstance());
289289
}
290-
}
290+
}

common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ImageManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ private ImageManager() {
4242
*
4343
* @return the image manager singleton.
4444
*/
45-
public static ImageManager getInstance() {
45+
public static synchronized ImageManager getInstance() {
4646
if (imageManager == null) {
4747
try {
4848
imageManager = new ImageManager();

0 commit comments

Comments
 (0)