Skip to content
This repository was archived by the owner on Jul 7, 2020. It is now read-only.

squid:S2259 - Null pointers should not be dereferenced #200

Open
georgekankava wants to merge 1 commit into
base: master
Choose a base branch
from
Open

squid:S2259 - Null pointers should not be dereferenced #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion hydra-main/src/main/java/com/addthis/hydra/job/JobExpand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,14 @@ static void registerExpander(String macroName, Class<? extends JobConfigExpander
} catch (InstantiationException | IllegalAccessException e) {
log.warn("Class '" + clazz + "' registered for '" + macroName + "' cannot be initialized: " + e, e);
} catch (ClassCastException e) {
log.warn("Class '" + clazz + "' registered for '" + macroName + "' is not JobConfigExpander but '" + o.getClass() + "'");
log.warn("Class '" + clazz + "' registered for '" + macroName + "' is not JobConfigExpander but '" + getObjectClassIfNotNull(o) + "'");
Copy link
Contributor

@yuesong yuesong Mar 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o is guaranteed to be not null, so the extra guard is unnecessary here.

Copy link
Contributor

@tea-dragon tea-dragon Apr 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to split the try block into two. A bit more verbose, but it would be clearer which catch blocks are guarding which line. Something something clarity is great.

It's been an awfully long time, so I suggest closing this PR (with gratitude), and then fixing whatever you want from a cleaner commit.

}
}

private static String getObjectClassIfNotNull(Object o) {
return o != null ? o.getClass().toString() : null;
}

private static String macroTemplateParamsHelper(String input, final HashMap<String, String> map)
throws TokenReplacerOverflowException {
return new TokenReplacer("%[", "]%") {
Expand Down
2 changes: 1 addition & 1 deletion hydra-main/src/main/java/com/addthis/hydra/job/spawn/balancer/JobTaskItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public JobTask getTask() {

@Override
public boolean equals(Object o) {
if (o.getClass() != getClass()) {
if (o == null || o.getClass() != getClass()) {
Copy link
Contributor

@tea-dragon tea-dragon Apr 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fun alternative here is a humble instanceof call. It'll return false for both nulls and guys two lines away from a class cast exception.

One time I checked the jvm bytecode specifically to spite an old professor and at the time at least, it was also several instructions more efficient.

return false;
}
JobTaskItem item2 = (JobTaskItem) o;
Expand Down