Why am i getting Possible null pointer dereference in findbug?

Question

On the 5th line of below code spotted as a bug by the findbugs:

Possible null pointer dereference in com.xyz.common.CommonUtils.FolderNotEmpty(String) due to return value of called method [Troubling(13), Normal confidence]

public static boolean FolderNotEmpty(String path) {
        boolean flag = false;
        File f = new File(path);
        if (f.isDirectory()) {
            if (f.listFiles().length > 0) {
                flag = true;
                LOGGER.info("Folder - " + path + " is not empty.");
            } else {
                LOGGER.info("Folder - " + path + " is empty.");
            }
        } else {
            LOGGER.warn("The given path is not a directory - " + path);
        }
        return flag;
    }

Answer 1

You have a race condition:

1. You call f.isDirectory(), which returns true.
2. I replace the directory at path with some ordinary file.
3. You call f.listFiles(), which returns null.

To avoid this, say File[] files = f.listFiles(); unconditionally, and then change your if to if (files != null). Better yet, reduce the nesting in your method this way:

public static boolean folderIsNotEmpty(String path) {
    File f = new File(path);
    File[] files = f.listFiles();

    if (files == null) {
        logger.warn("not a directory");
        return false;
    }

    if (files.length > 0) { 
        logger.info("not empty");
        return true;
    } else {
        logger.info("empty");
        return false;
    }
}

(Or, if you don't need the log statements, return (files.length > 0).)

Answer 2

Because f.listFiles() can return null. Rewrite it with following code:

if (f.listFiles() != null && f.listFiles().length > 0)

Answer 3

listFiles method of File class can return null. So you need to check if f.listFiles() return null at 5th line, otherwise, if (f.listFiles().length > 0) can cause NPE.

Answer 4

Actually, your code is perfectly safe.

If this abstract pathname does not denote a directory, then this method returns null. Otherwise an array of File objects is returned, one for each file or directory in the directory.

That is exactly what you have covered.

However, Findbugs cannot know about that contract. It just says there is a potential NPE. You can choose to ignore that.