[
https://issues.apache.org/jira/browse/HDFS-12915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16294575#comment-16294575
]
SammiChen commented on HDFS-12915:
----------------------------------
Thanks [~jojochuang] for reporting and working on it. Back to the review
period of HDFS-12840, I do double checked the findbugs report and find nothing
suspicious. It's a false alert. I think Eddy did the same check.
I dived a little deeper this time.
This piece of code which will trigger findbugs to alert warning.
{noformat}
Preconditions.checkArgument(replication != null && replication >= 0 &&
replication <= MAX_REDUNDANCY,
"Invalid replication value " + replication);
{noformat}
This piece of code will not trigger warning.
{noformat}
Preconditions.checkArgument(replication != null && replication >= 0 &&
replication <= MAX_REDUNDANCY,
"Invalid replication value " + replication);
{noformat}
The only difference is the condition clause is in one line in the second code.
When the condition clause is separated into two lines, findbugs cannot handle
the case correctly, will trigger the false alert.
And [~chris.douglas], Preconditions does support formatted strings.
So basically I think Preconditions is very useful and neat to check parameters.
But if the check condition is complex, to not trigger the annoying findbugs
warning, a traditional {{if()}} plus {{throw}} statement seems more fit.
By the way, the last update of the findbugs web site is Mar. 2015. Seems it's
lack of maintenance these days.
For the patch, the following statement is not appropriate. If
{{erasureCodingPolicyID}} is null and {{blockType}} is stripped, it should
throw exception. {{REPLICATION_POLICY_ID}} is a special EC policy. It
represents the "3 replica" scheme. File with this policy is effectively 3
replica file, not EC file.
{noformat}
if (null == erasureCodingPolicyID) {
erasureCodingPolicyID = REPLICATION_POLICY_ID;
}
{noformat}
> Fix findbugs warning in INodeFile$HeaderFormat.getBlockLayoutRedundancy
> -----------------------------------------------------------------------
>
> Key: HDFS-12915
> URL: https://issues.apache.org/jira/browse/HDFS-12915
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: namenode
> Affects Versions: 3.0.0
> Reporter: Wei-Chiu Chuang
> Attachments: HDFS-12915.00.patch, HDFS-12915.01.patch
>
>
> It seems HDFS-12840 creates a new findbugs warning.
> Possible null pointer dereference of replication in
> org.apache.hadoop.hdfs.server.namenode.INodeFile$HeaderFormat.getBlockLayoutRedundancy(BlockType,
> Short, Byte)
> Bug type NP_NULL_ON_SOME_PATH (click for details)
> In class org.apache.hadoop.hdfs.server.namenode.INodeFile$HeaderFormat
> In method
> org.apache.hadoop.hdfs.server.namenode.INodeFile$HeaderFormat.getBlockLayoutRedundancy(BlockType,
> Short, Byte)
> Value loaded from replication
> Dereferenced at INodeFile.java:[line 210]
> Known null at INodeFile.java:[line 207]
> From a quick look at the patch, it seems bogus though. [~eddyxu][~Sammi]
> would you please double check?
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
