Skip to content

Fix the 117 PMD violations and gate pmd:check at verify (#114) #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bertysentry merged 2 commits into from
Oct 7, 2026
Merged

Fix the 117 PMD violations and gate pmd:check at verify (#114) #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70a1fde
Fix the 117 PMD violations and gate pmd:check at verify (#114)
bertysentry Oct 7, 2026
b08a819
Restore the interrupt flag only once a sessionless tag is found (#114)
bertysentry Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Unit tests must not depend on a real BMC. Exercising the RMCP+ session code agai

## Code quality reports

Code quality reports (checkstyle, pmd/cpd, spotbugs) are generated by `mvn verify site` into ./target/checkstyle-result.xml, ./target/pmd.xml, ./target/cpd.xml and ./target/spotbugsXml.xml. Checkstyle is gated (the build fails on any error); PMD/CPD and SpotBugs are not yet gated (issues #114, #115, #116 track the clean-up). Do not add new violations: check the reports for the files you changed before committing and submitting your code. On JDK 21+ the SpotBugs plugin version inherited from the parent POM cannot read the JDK class files; run `mvn com.github.spotbugs:spotbugs-maven-plugin:4.10.4.1:spotbugs` instead.
Code quality reports (checkstyle, pmd/cpd, spotbugs) are generated by `mvn verify site` into ./target/checkstyle-result.xml, ./target/pmd.xml, ./target/cpd.xml and ./target/spotbugsXml.xml. Checkstyle and PMD are gated (the build fails on any error); CPD and SpotBugs are not yet gated (issues #115 and #116 track the clean-up). Do not add new violations: check the reports for the files you changed before committing and submitting your code. On JDK 21+ the SpotBugs plugin version inherited from the parent POM cannot read the JDK class files; run `mvn com.github.spotbugs:spotbugs-maven-plugin:4.10.4.1:spotbugs` instead.

## Documentation

Expand Down
24 changes: 23 additions & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,27 @@
</executions>
</plugin>

<!-- pmd: fail the build on any violation of pmd.xml, so the PMD report stays clean (cpd-check awaits #115) -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>3.28.0</version>
<configuration>
<targetJdk>${maven.compiler.release}</targetJdk>
<rulesets>
<ruleset>pmd.xml</ruleset>
</rulesets>
<printFailingErrors>true</printFailingErrors>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>

<!-- license -->
<plugin>
<groupId>org.codehaus.mojo</groupId>
Expand All @@ -162,9 +183,10 @@
<reporting>
<plugins>

<!-- pmd -->
<!-- pmd: same version as the build gate, so the report shows what the gate checked -->
<plugin>
<artifactId>maven-pmd-plugin</artifactId>
<version>3.28.0</version>
<configuration>
<linkXref>true</linkXref>
<sourceEncoding>${project.build.sourceEncoding}</sourceEncoding>
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public void close() {
try {
connector.closeSession(handle);
} catch (Exception e) {
// Ignore
LOGGER.debug("Failed to close the IPMI session", e);
}
}

Expand Down
10 changes: 7 additions & 3 deletions src/main/java/org/metricshub/ipmi/client/runner/GetFrusRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,16 @@
import org.metricshub.ipmi.core.coding.payload.CompletionCode;
import org.metricshub.ipmi.core.coding.payload.lan.IPMIException;
import org.metricshub.ipmi.core.coding.protocol.AuthenticationType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Get FRU information
*/
public class GetFrusRunner extends AbstractIpmiRunner<List<Fru>> {

private static final Logger LOGGER = LoggerFactory.getLogger(GetFrusRunner.class);

/**
* Id of the built-in, default FRU
*/
Expand Down Expand Up @@ -197,7 +201,7 @@ private void processFruRecord(
}

} catch (IPMIException e) {
// Nothing can be done
LOGGER.warn("Failed to read the FRU of sensor record {}: {}", sensorRecord.getId(), e.getMessage());
}

}
Expand Down Expand Up @@ -249,7 +253,7 @@ private List<FruRecord> getFruRecords(int fruId) throws Exception {
fruData.add(data);

} catch (Exception e) {
// Nothing can be done
LOGGER.warn("Failed to read FRU {} at offset {}, the FRU data will be truncated: {}", fruId, i, e.getMessage());
}
}

Expand All @@ -265,7 +269,7 @@ private List<FruRecord> getFruRecords(int fruId) throws Exception {
.collect(Collectors.toList());

} catch (Exception e) {
// Nothing can be done
LOGGER.warn("Failed to decode FRU {}: {}", fruId, e.getMessage());
}

return new ArrayList<>();
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/metricshub/ipmi/core/api/async/IpmiAsyncConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@
* Loads properties from the properties file.
*/
private void loadProperties() {
retries = Integer.parseInt(PropertiesManager.getInstance().getProperty("retries"));

Check warning on line 157 in src/main/java/org/metricshub/ipmi/core/api/async/IpmiAsyncConnector.java

View workflow job for this annotation

GitHub Actions / spotbugs

AT_STALE_THREAD_WRITE_OF_PRIMITIVE 

Shared primitive variable "retries" in one thread may not yield the value of the most recent write from another thread
Raw output 
 SEI CERT rule VNA00-J [https://wiki.sei.cmu.edu/confluence/display/java/VNA00-J.+Ensure+visibility+when+accessing+shared+primitive+variables]describes that reading a shared primitive variable in one thread may not yield the value of the most recent write to the variable from another thread. Consequently, the thread may observe a stale value of the shared variable. 

To fix it, declare the variable volatile, change the type of the field to the corresponding atomic type from java.lang.concurrent.atomic or correctly synchronize the code. Declaring the variable volatile may not be enough in some cases: e.g. when the variable is assigned a value which depends on the current value or on the result of nonatomic compound operations. This guarantees that 64-bit primitive long and double variables are accessed atomically.
}

/**
Expand Down Expand Up @@ -388,7 +388,6 @@
}
}
}
return;
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
* Existing session that should be reused (if possible) for SOL communication.
*/
public SerialOverLan(IpmiConnector connector, Session session) throws SOLException, SessionException {
this.connector = connector;

Check warning on line 146 in src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP2 

new org.metricshub.ipmi.core.api.sol.SerialOverLan(IpmiConnector, Session) may expose internal representation by storing an externally mutable object into SerialOverLan.connector
Raw output 
 This code stores a reference to an externally mutable object into the internal representation of the object. If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

int solPayloadPort = activatePayload(connector, session.getConnectionHandle());

Expand Down Expand Up @@ -241,7 +241,7 @@
connectionHandle,
activatePayload);

this.maxPayloadSize = activatePayloadResponseData.getInboundPayloadSize();

Check warning on line 244 in src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java

View workflow job for this annotation

GitHub Actions / spotbugs

AT_STALE_THREAD_WRITE_OF_PRIMITIVE 

Shared primitive variable "maxPayloadSize" in one thread may not yield the value of the most recent write from another thread
Raw output 
 SEI CERT rule VNA00-J [https://wiki.sei.cmu.edu/confluence/display/java/VNA00-J.+Ensure+visibility+when+accessing+shared+primitive+variables]describes that reading a shared primitive variable in one thread may not yield the value of the most recent write to the variable from another thread. Consequently, the thread may observe a stale value of the shared variable. 

To fix it, declare the variable volatile, change the type of the field to the corresponding atomic type from java.lang.concurrent.atomic or correctly synchronize the code. Declaring the variable volatile may not be enough in some cases: e.g. when the variable is assigned a value which depends on the current value or on the result of nonatomic compound operations. This guarantees that 64-bit primitive long and double variables are accessed atomically.

return activatePayloadResponseData.getPayloadUdpPortNumber();

Expand Down Expand Up @@ -413,7 +413,7 @@
* @return true if whole string was successfully sent and acknowledged by remote server, false otherwise.
*/
public boolean writeString(String string) {
return writeBytes(string.getBytes());

Check warning on line 416 in src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in org.metricshub.ipmi.core.api.sol.SerialOverLan.writeString(String): String.getBytes()
Raw output 
 Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
}

/**
Expand Down Expand Up @@ -541,7 +541,7 @@
* @return all bytes that could be read as {@link String}, but no more than given byteCount.
*/
public String readString(int byteCount) {
return new String(readBytes(byteCount));

Check warning on line 544 in src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in org.metricshub.ipmi.core.api.sol.SerialOverLan.readString(int): new String(byte[])
Raw output 
 Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
}

/**
Expand Down Expand Up @@ -611,7 +611,12 @@
long startTime = System.currentTimeMillis();

while (isTooFewBytesAvailable(wantedByteCount) && timeoutNotHit(timeout, startTime)) {
// NOP, just waiting
try {
Thread.sleep(1);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return;
}
}
}

Expand Down
Loading
Loading