Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: neutral (asf.osuosl.org: local policy)
Message-ID: <448A2A32.6010200@pearsoncmg.com>
Date: Fri, 09 Jun 2006 22:10:58 -0400
From: Chris Darroch <chrisd@pearsoncmg.com>
Organization: Pearson CTG/CMG
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060423
MIME-Version: 1.0
To: dev@apr.apache.org
Subject: [PATCH] apr_dbd_oracle.c query/select fixes #37664
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi --

   Maybe someone with APR commit access (Nick, Bojan?) could review
the patch I've attached to PR 37664 and commit it to trunk; it has a
variety of fixes and cleanups for apr_dbd_oracle.c.  It's been a while
since I touched that PR but I'm sort of holding it open for future
Oracle driver patches; let me know if you want to close it.  Thanks!

   From the PR notes:

   This patch file contains a variety of fixes and cleanups, mostly
related to the prepare function and the various query and select
functions.

   The pvquery and pvselect functions were looking for true int and double
arguments in the va_list args.	This is not how other drivers function; for
example, the pgsql and sqlite3 drivers assume all va_list arguments as strings.  This is in keeping with the pquery and pselect functions too.
So, make the Oracle driver do the same.

   The OCIStmtRelease() function should only be used if Oracle statement
handles have been prepared with OCIStmtPrepare2(), which is only available
in Oracle 9.2 and above.  For now, stick with just OCIHandleFree() and put
the stuff related to OCIStmtRelease() into unused #ifdef blocks in the
freeStatement() and freeStatements() APR memory pool callback cleanup
functions.  Note that when OCIStmtRelease() is used in the future,
OCIHandleFree() should not be used because OCIStmtRelease() will do that
for you.

   The freeStatement() callback cleanup function should return apr_status_t
to match the return value expected by apr_pool_cleanup_register() and
friends.

   The freeStatement() callback cleanup function should also use a private
Oracle error handle like the freeStatements() one (if and when it uses
OCIStmtRelease() in the future).  Otherwise, if an error occurs during
the execution of dbd_oracle_query(), then the call to apr_pool_destroy()
on the private pool causes freeStatement() to be called, which if it
then encounters its own error, would then overwrite the previous error data
in the Oracle handle.  That in turn means the caller can't access the
original error message that caused apr_dbd_query() to fail.

   In all #ifdef DEBUG blocks, never overwrite sql->status.  Otherwise the
debugging code changes the error values and messages output, which isn't
really what you want debugging code to do; instead, it should be non-invasive.

   In dbd_oracle_prepare(), make sure to set stmt->nargs to 0 before counting
arguments in the SQL query.  Otherwise, if the caller passes a non-NULL
statement pointer (i.e., a previously allocated apr_dbd_prepared_t structure),
we miscalculate the number of expected arguments to p[v]query/select().
This usually causes a crash when the subsequent calls to p[v]query/select()
are made.

   Finally, remove some non-functional BLOB/CLOB trial code from pvquery()
and make it similar to the other three p[v]query/select() functions in
this regard.

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B