Re: [PATCHES] libpq events patch (with sgml docs)

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Andrew Chernow <ac(at)esilo(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Merlin Moncure <mmoncure(at)gmail(dot)com>
Subject: Re: [PATCHES] libpq events patch (with sgml docs)
Date: 2008-09-05 19:48:53
Message-ID: 20080905194853.GL4353@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Martin Pihlak 2008-09-05 19:51:36 Re: reducing statistics write overhead
Previous Message Andrew Chernow 2008-09-05 19:40:56 Re: [PATCHES] libpq events patch (with sgml docs)

Browse pgsql-patches by date

  From Date Subject
Next Message Andrew Chernow 2008-09-05 20:08:07 Re: [PATCHES] libpq events patch (with sgml docs)
Previous Message Andrew Chernow 2008-09-05 19:40:56 Re: [PATCHES] libpq events patch (with sgml docs)