[
https://issues.apache.org/jira/browse/DIRKRB-368?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14628021#comment-14628021
]
Lin Chen commented on DIRKRB-368:
---------------------------------
Hi yaning, some question:
1.
{code}
if (cacheFile.setWritable(true, true)) {
return;
}
{code}
Maybe it is better to judge the {{false}} condition here, just like
{code}
if ( !cacheFile.setWritable(true, true) ) {
System.err.println(...) or throw exceptions..
}
{code}
2. It seems to be something wrong with the main method of {{KadminTool}}:
{code}
try (Scanner scanner = new Scanner(System.in)) {
String input = scanner.nextLine();
boolean quit = input.equals("quit") || input.equals("exit") ||
input.equals("q");
while (!quit) {
execute(kadmin, input);
System.out.print(PROMPT + ": ");
input = scanner.nextLine();
}
}
{code}
The while loop is an infinite loop..
3. In {{ListPrincipalCommand}},
{code}
System.out.println("Principals are listed:");
+ if (principalNames == null) {
+ return;
+ }
for (String principalName : principalNames) {
System.out.println(principalName);
}
{code}
It is better to be written as:
{code}
if (principalNames != null) {
System.out.println("Principals are listed:");
for (String principalName : principalNames) {
System.out.println(principalName);
}
}
{code}
Since if principalNames is equal to null, "Fail to list principal!" will be
printed.
> Fix findbugs Problems for kerby-tool module
> -------------------------------------------
>
> Key: DIRKRB-368
> URL: https://issues.apache.org/jira/browse/DIRKRB-368
> Project: Directory Kerberos
> Issue Type: Sub-task
> Reporter: Xu Yaning
> Assignee: Xu Yaning
> Attachments: DIRKRB-368-v1.patch, DIRKRB-368-v2.patch
>
>
> Findbugs maven plugin reports the following problems:
> # *org.apache.kerby.kerberos.tool.kinit.KinitOption.setDescription(String)*
> unconditionally sets the field description;
> # *org.apache.kerby.kerberos.tool.kinit.KinitOption.setName(String)*
> unconditionally sets the field name;
> # *org.apache.kerby.kerberos.tool.kinit.KinitOption.setType(KOptionType)*
> unconditionally sets the field type;
> # *org.apache.kerby.kerberos.tool.kinit.KinitOption.setValue(Object)*
> unconditionally sets the field value;
> # *org.apache.kerby.kerberos.tool.klist.KlistOption.setDescription(String)*
> unconditionally sets the field description;
> # *org.apache.kerby.kerberos.tool.klist.KlistOption.setName(String)*
> unconditionally sets the field name;
> # *org.apache.kerby.kerberos.tool.klist.KlistOption.setType(KOptionType)*
> unconditionally sets the field type;
> # *org.apache.kerby.kerberos.tool.klist.KlistOption.setValue(Object)*
> unconditionally sets the field value.
> # Dead store to error in
> *org.apache.kerby.kerberos.tool.kinit.KinitTool.main(String[])*;
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.kinit.KinitTool.getPassword(String)*: new
> java.util.Scanner(InputStream);
> #
> *org.apache.kerby.kerberos.tool.klist.KlistTool.printCredentialCacheInfo(KOptions)*
> may fail to clean up java.io.InputStream;
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.token.TokenCache.readToken(String)*: new
> java.io.FileReader(File);
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.token.TokenCache.writeToken(String)*: new
> java.io.FileWriter(File);
> # *org.apache.kerby.kerberos.tool.token.TokenCache.writeToken(String)* may
> fail to clean up java.io.Writer on checked exception;
> # Exceptional return value of java.io.File.delete() ignored in
> *org.apache.kerby.kerberos.tool.token.TokenCache.writeToken(String)*;
> # Exceptional return value of java.io.File.setWritable(boolean, boolean)
> ignored in
> *org.apache.kerby.kerberos.tool.token.TokenCache.writeToken(String)*;
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.kadmin.KadminTool.main(String[])*: new
> java.util.Scanner(InputStream);
> # There is an apparent infinite loop in
> *org.apache.kerby.kerberos.tool.kadmin.KadminTool.main(String[])*;
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.kadmin.ToolUtil.getReplay(String)*: new
> java.util.Scanner(InputStream);
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.kadmin.command.AddPrincipalCommand.getPassword(String)*:
> new java.util.Scanner(InputStream);
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.kadmin.command.ChangePasswordCommand.getPassword(String)*:
> new java.util.Scanner(InputStream);
> # Found reliance on default encoding in
> *org.apache.kerby.kerberos.tool.kadmin.command.DeletePrincipalCommand.execute(String)*:
> new java.util.Scanner(InputStream);
> # Possible null pointer dereference of principalNames in
> *org.apache.kerby.kerberos.tool.kadmin.command.ListPrincipalCommand.execute(String)*;
> # Dead store to error in
> *org.apache.kerby.kerberos.tool.kadmin.command.ModifyPrincipalCommand.parseOptions(String[])*;
> # Read of unwritten field kOptions in
> *org.apache.kerby.kerberos.tool.kadmin.command.ModifyPrincipalCommand.parseOptions(String[])*;
> # Unwritten field:
> *org.apache.kerby.kerberos.tool.kadmin.command.ModifyPrincipalCommand.kOptions*;
> Problems 1 to 8 are required to be fixed in DIRKRB-367, the others are to be
> fixed here.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
