Message-ID: <48D0E0E8.5070301@esilo.com>
Date: Wed, 17 Sep 2008 06:50:16 -0400
From: Andrew Chernow <ac@esilo.com>
User-Agent: Thunderbird 2.0.0.16 (Windows/20080708)
MIME-Version: 1.0
To: Tom Lane <tgl@sss.pgh.pa.us>
CC: Alvaro Herrera <alvherre@commandprompt.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Merlin Moncure <mmoncure@gmail.com>
Subject: Re: [PATCHES] libpq events patch (with sgml docs)
References: <48C18533.2000204@esilo.com>
	<20080905192215.GK4353@alvh.no-ip.org> <48C18B48.509@esilo.com>
	<20080905194853.GL4353@alvh.no-ip.org> <48C191A7.3000205@esilo.com>
	<20080905201504.GM4353@alvh.no-ip.org>
	<48C19759.90402@esilo.com> <27615.1221619326@sss.pgh.pa.us>
	<48D072E3.6010703@esilo.com> <28673.1221622817@sss.pgh.pa.us>
	<48D080B0.6070104@esilo.com> <454.1221626213@sss.pgh.pa.us>
	<48D0DFBE.9050900@esilo.com>
In-Reply-To: <48D0DFBE.9050900@esilo.com>
Content-Type: multipart/mixed; boundary="------------010602010000040308050308"

This is a multi-part message in MIME format.
--------------010602010000040308050308
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Andrew Chernow wrote:
>  > Now, it's questionable how tense we need to be about that as long as
>  > event proc failure aborts calling of later event procs.  That means
>  > that procs have to be robust against getting DESTROY with no CREATE
>  > calls in any case.  Should we try to make that less uncertain?
>  >
>  >
> 
> I attached a patch that adds a 'needDestroy' member to PGEvent  It is 
> set when resultcreate or resultcopy succeeds and checked during a 
> PQclear.  That *should* resolve the issue of "no resultcreate but gets a 
> resultdestroy".
> 
> 

Shoot.  I have a booboo in that last patch.  Attached is the corrected version.

-- 
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

--------------010602010000040308050308
Content-Type: text/x-patch;
 name="pgevent.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="pgevent.patch"

Index: src/interfaces/libpq/fe-exec.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.198
diff -C6 -r1.198 fe-exec.c
*** src/interfaces/libpq/fe-exec.c	17 Sep 2008 04:31:08 -0000	1.198
--- src/interfaces/libpq/fe-exec.c	17 Sep 2008 10:49:10 -0000
***************
*** 356,367 ****
--- 356,369 ----
  		if (!dest->events[i].proc(PGEVT_RESULTCOPY, &evt,
  								  dest->events[i].passThrough))
  		{
  			PQclear(dest);
  			return NULL;
  		}
+ 
+ 		dest->events[i].needDestroy = TRUE;
  	}
  
  	return dest;
  }
  
  /*
***************
*** 378,396 ****
  		return NULL;
  
  	newEvents = (PGEvent *) malloc(count * sizeof(PGEvent));
  	if (!newEvents)
  		return NULL;
  
- 	memcpy(newEvents, events, count * sizeof(PGEvent));
- 
- 	/* NULL out the data pointers and deep copy names */
  	for (i = 0; i < count; i++)
  	{
  		newEvents[i].data = NULL;
! 		newEvents[i].name = strdup(newEvents[i].name);
  		if (!newEvents[i].name)
  		{
  			while (--i >= 0)
  				free(newEvents[i].name);
  			free(newEvents);
  			return NULL;
--- 380,398 ----
  		return NULL;
  
  	newEvents = (PGEvent *) malloc(count * sizeof(PGEvent));
  	if (!newEvents)
  		return NULL;
  
  	for (i = 0; i < count; i++)
  	{
+ 		newEvents[i].proc = events[i].proc;
+ 		newEvents[i].passThrough = events[i].passThrough;
+ 		newEvents[i].needDestroy = FALSE;
  		newEvents[i].data = NULL;
! 		newEvents[i].name = strdup(events[i].name);
  		if (!newEvents[i].name)
  		{
  			while (--i >= 0)
  				free(newEvents[i].name);
  			free(newEvents);
  			return NULL;
***************
*** 663,679 ****
  
  	if (!res)
  		return;
  
  	for (i = 0; i < res->nEvents; i++)
  	{
! 		PGEventResultDestroy evt;
  
- 		evt.result = res;
- 		(void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt,
- 								   res->events[i].passThrough);
  		free(res->events[i].name);
  	}
  
  	if (res->events)
  		free(res->events);
  
--- 665,685 ----
  
  	if (!res)
  		return;
  
  	for (i = 0; i < res->nEvents; i++)
  	{
! 		if(res->events[i].needDestroy)
! 		{
! 			PGEventResultDestroy evt;
! 
! 			evt.result = res;
! 			(void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt,
! 									   res->events[i].passThrough);
! 		}
  
  		free(res->events[i].name);
  	}
  
  	if (res->events)
  		free(res->events);
  
***************
*** 1609,1620 ****
--- 1615,1628 ----
  								  libpq_gettext("PGEventProc \"%s\" failed during PGEVT_RESULTCREATE event\n"),
  								  res->events[i].name);
  				pqSetResultError(res, conn->errorMessage.data);
  				res->resultStatus = PGRES_FATAL_ERROR;
  				break;
  			}
+ 
+ 			res->events[i].needDestroy = TRUE;
  		}
  	}
  
  	return res;
  }
  
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.132
diff -C6 -r1.132 libpq-int.h
*** src/interfaces/libpq/libpq-int.h	17 Sep 2008 04:31:08 -0000	1.132
--- src/interfaces/libpq/libpq-int.h	17 Sep 2008 10:49:10 -0000
***************
*** 153,164 ****
--- 153,165 ----
  typedef struct PGEvent
  {
  	PGEventProc	proc;			/* the function to call on events */
  	char	   *name;			/* used only for error messages */
  	void	   *passThrough;	/* pointer supplied at registration time */
  	void	   *data;			/* optional state (instance) data */
+ 	int		   needDestroy; 	/* indicates a PGEVT_RESULTDESTROY is needed. */
  } PGEvent;
  
  struct pg_result
  {
  	int			ntups;
  	int			numAttributes;

--------------010602010000040308050308--