From: Alvaro Herrera <alvherre@commandprompt.com>
To: Andrew Chernow <ac@esilo.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Merlin Moncure <mmoncure@gmail.com>
Subject: Re: [PATCHES] libpq events patch (with sgml docs)
Date: Fri, 5 Sep 2008 15:48:53 -0400
Message-ID: <20080905194853.GL4353@alvh.no-ip.org> (raw)
In-Reply-To: <48C18B48.509@esilo.com>
References: <48C18533.2000204@esilo.com>
<20080905192215.GK4353@alvh.no-ip.org>
<48C18B48.509@esilo.com>
Andrew Chernow escribió:
> /*
> * PQmakeEmptyPGresult
> * returns a newly allocated, initialized PGresult with given status.
> * If conn is not NULL and status indicates an error, the conn's
> * errorMessage is copied.
> *
> * Note this is exported --- you wouldn't think an application would need
> * to build its own PGresults, but this has proven useful in both libpgtcl
> * and the Perl5 interface, so maybe it's not so unreasonable.
> + *
> + * Updated April 2008 - If conn is not NULL, event states will be copied
> + * from the PGconn to the created PGresult.
> */
Don't do this either. We don't really need to know when the thing was
changed; the comment should just state what the function does. I had
folded the last paragraph into the introductory one, but I think you
lost that part of my change.
> + /* resultalloc the attribute names. The above memcpy has the attr
> + * names pointing at the callers provided attDescs memory.
> + */
"resultalloc"? Why not just "allocate"?
> * PQclear -
> * free's the memory associated with a PGresult
> */
I'd add a comment here stating why the event name is not deallocated;
otherwise it just looks like it's being leaked.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: alvherre@commandprompt.com, ac@esilo.com, pgsql-hackers@postgresql.org, mmoncure@gmail.com
Subject: Re: [PATCHES] libpq events patch (with sgml docs)
In-Reply-To: <20080905194853.GL4353@alvh.no-ip.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox