Re: [PATCHES] libpq events patch (with sgml docs) - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: [PATCHES] libpq events patch (with sgml docs)
Date
Msg-id 20080905194853.GL4353@alvh.no-ip.org
Whole thread Raw
In response to Re: [PATCHES] libpq events patch (with sgml docs)  (Andrew Chernow <ac@esilo.com>)
Responses Re: [PATCHES] libpq events patch (with sgml docs)  (Andrew Chernow <ac@esilo.com>)
List pgsql-hackers
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.


pgsql-hackers by date:

Previous
From: Andrew Chernow
Date:
Subject: Re: [PATCHES] libpq events patch (with sgml docs)
Next
From: Martin Pihlak
Date:
Subject: Re: reducing statistics write overhead