Resource leak or false positive-CodePudding

I have a code like this:

std::string getInfo(FILE *fp)
{
    char buffer[30];
    if (fread(buffer, 19, 1, fp) == 1)
        buffer[19] = '\0';
    else
        buffer[0] = '\0';

    return buffer;
}

I'm using cppcheck for static analysis and it spits a warning:

error: Resource leak: fp [resourceLeak]
 return buffer;
 ^

The way I see it, since the return is by value, the data will be copied from "buffer" into std::string's storage, so there's no leak there.

Does this pose some real problem or is it a false positive?

CodePudding user response：

error: Resource leak: fp [resourceLeak]

You are not leaking any resource from this function. So, if you're only analyzing this function, this is a bug in your cppcheck. However, as @Taekahn points out - maybe the leak is elsewhere in your program.

Also, since you're returning std::string, you can use a constructor taking a pointer and a count, and not bother with the '\0' in the buffer:

std::string getInfo(FILE *fp)
{
    static constexpr const std::size_t info_length { 19 };
    char buffer[info_length];
    if (fread(buffer, info_length, 1, fp) == 1)
        return {buffer, info_length};
    else
        return {};
}

If you're using C 23, you might consider returning an std::expected instead of the empty string, e.g. something like:

std::expected<std::string, errc> getInfo(FILE *fp)
{
    static constexpr const std::size_t info_length { 19 };
    char buffer[info_length];
    if (fread(buffer, info_length, 1, fp) == 1)
        return std::string{buffer, info_length};
    else
        return std::unexpected{errno};
}

CodePudding user response：

There is no leak in the shown function.

The std::FILE* provided as the argument may have been created using std::fopen, and such resource may leak if it isn't closed at some point. But since this function doesn't acquire the resource, this function shouldn't be responsible for releasing it either.

In conclusion, it's either a false positive or misleading diagnostic. At best (from point of view of the diagnostic), there may be a leak somewhere else, although that's not demonstrated within the example.

P.S. Since you return a std::string, you can avoid copying the content first into a separate buffer by reading directly into the std::string:

constexpr std::size_t count = 19;
std::string buffer(count, '\0');
std::size_t r = fread(buffer.data(), count, 1, fp);
if (r)
    return buffer;
else
    return {};