[ 
https://issues.apache.org/jira/browse/DIRKRB-368?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14629082#comment-14629082
 ] 

Xu Yaning edited comment on DIRKRB-368 at 7/16/15 2:15 AM:
-----------------------------------------------------------

Thanks for Lin's review! As for the second suggestion, I remember I've added 
{code}
quit = input.equals("quit") || input.equals("exit") || input.equals("q");
{code}
in the end of while loop:
{code}
 try (Scanner scanner = new Scanner(System.in, "UTF-8")) {
            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();
                quit = input.equals("quit") || input.equals("exit") || 
input.equals("q");
            }
        }
{code}

As for the third suggestion, I think the following may be better:
{code}
public void execute(String input) {
        String[] commands = input.split(" ");

        if (commands.length == 1) {
            try {
                List<String> principalNames = getKadmin().getPrincipals();
                System.out.println("Principals are listed:");
                for (String principalName : principalNames) {
                    System.out.println(principalName);
                }
            } catch (KrbException e) {
                System.err.print("Fail to list principal!" + e.getMessage());
            }
        }
    }
{code}


was (Author: yaningxu):
Thanks for Lin's review! As for the second suggestion, I remember I've add 
{code}
quit = input.equals("quit") || input.equals("exit") || input.equals("q");
{code}
in the end of while loop:
{code}
 try (Scanner scanner = new Scanner(System.in, "UTF-8")) {
            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();
                quit = input.equals("quit") || input.equals("exit") || 
input.equals("q");
            }
        }
{code}

As for the third suggestion, I think the following may be better:
{code}
public void execute(String input) {
        String[] commands = input.split(" ");

        if (commands.length == 1) {
            try {
                List<String> principalNames = getKadmin().getPrincipals();
                System.out.println("Principals are listed:");
                for (String principalName : principalNames) {
                    System.out.println(principalName);
                }
            } catch (KrbException e) {
                System.err.print("Fail to list principal!" + e.getMessage());
            }
        }
    }
{code}

> 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)

Reply via email to