Subject: Re: How to properly fix memory leak

From: "David G. Johnston" <[email-masked]>
Date: Fri, 25 Apr 2025 20:48:00 -0700

On Friday, April 25, 2025, Igor Korot <[email-masked]> wrote:

>
>         for( int i = 0; i < PQntuples( res ); i++ )
>         {
>             auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
> res, i, 1 ) );
>             m_tablespaces.push_back( temp1 );
>         } // this line gives a leak according to VLD
>     }
>     PQclear( res );
>     return result;
> [/code]
>
> I ran this code on MSVC 2017  with VLD and according to the VLD report I
> have
> a memory leak on the line indicated.

Seems like a false positive.

>
> Should I call PQclear() on every iteration of the loop?
>

Would make processing more than a single row impossible if you throw away
the result after processing one row.

David J.

---

From: Igor Korot <[email-masked]>
Date: Fri, 25 Apr 2025 23:17:17 -0500

Hi, David,

On Fri, Apr 25, 2025 at 10:48 PM David G. Johnston
<[email-masked]> wrote:
>> I ran this code on MSVC 2017  with VLD and according to the VLD report I have

And the error case was handled correctly, right?

Thank you.

> Would make processing more than a single row impossible if you throw away the result after processing one row.
>
> David J.
>

---

From: "David G. Johnston" <[email-masked]>
Date: Fri, 25 Apr 2025 21:55:14 -0700

> And the error case was handled correctly, right?

Seems like answering that requires knowing what the query is or can be.  I
also have no idea what idiomatic code looks like.  Though, I’d probably use
PQresultErrorMessage and check affirmatively for the tuples and error cases
and have a final else should the status be something unexpected.

David J.

---

From: Igor Korot <[email-masked]>
Date: Sat, 26 Apr 2025 00:12:59 -0500

Hi, David,

On Fri, Apr 25, 2025 at 11:55 PM David G. Johnston
<[email-masked]> wrote:
> Seems like answering that requires knowing what the query is or can be.  I also have no idea what idiomatic code looks like.  Though, I’d probably use PQresultErrorMessage and check affirmatively for the tuples and error cases and have a final else should the status be something unexpected.

Understood.

Below is the full function:

[code]
int PostgresDatabase::PopulateTablespaces(std::vector<std::wstring> &errorMsg)
{
    int result = 0;
    std::wstring errorMessage;
    std::wstring query = L"SELECT * FROM pg_tablespace;";
    auto res = PQexec( m_db, m_pimpl->m_myconv.to_bytes( query.c_str()
).c_str() );      /* ask for binary results */
    if( PQresultStatus( res ) != PGRES_TUPLES_OK )
    {
        auto err = m_pimpl->m_myconv.from_bytes( PQerrorMessage( m_db ) );
        errorMsg.push_back( L"Update validation table: " + err );
        result = 1;
    }
    else
    {
        for( int i = 0; i < PQntuples( res ); i++ )
        {
            auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
            m_tablespaces.push_back( temp1 );
        }
    }
    PQclear( res );
    return result;
}
[/code]

Thank you.

---

From: Igor Korot <[email-masked]>
Date: Sat, 26 Apr 2025 23:58:00 -0500

David et al,

> Seems like a false positive.

Looks like it is false positive.

I ran the code under valgrind and there I didn't get
such a leak.
I did however get a different issues which I fixed.
But even after moving the fixes ober to Windows and trying
to run it - I still see that.

For now I put this to bed as it is not an issue on Linux.

Thank you.