CODEWITHSUNDEEP
javafindbugs
rahul sharma
rahul sharma

Reputation: 509

Why am i getting Possible null pointer dereference in findbug?

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;
    }

Upvotes: 1

Views: 2618

Answers (4)

chrylis -cautiouslyoptimistic-
chrylis -cautiouslyoptimistic-

Reputation: 77226

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

Upvotes: 1

Dorian Gray
Dorian Gray

Reputation: 2981

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.

Upvotes: 0

Ali Gelenler
Ali Gelenler

Reputation: 241

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.

Upvotes: 0

Giga Kokaia
Giga Kokaia

Reputation: 939

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

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

Upvotes: 1

Related Questions