[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r36078 - in trunk/subversion: include/private libsvn_subr

From: Greg Stein <gstein_at_gmail.com>
Date: Tue, 24 Feb 2009 09:34:13 +0100

On Mon, Feb 23, 2009 at 23:03, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On Mon, Feb 23, 2009 at 7:34 PM, Greg Stein <gstein_at_gmail.com> wrote:
>> Author: gstein
>> Date: Mon Feb 23 08:34:29 2009
>> New Revision: 36078
>>
>> Log:
>> Add new binding/retrieval functions to our sqlite interface to deal with
>> svn datatypes.
>>
> [...]
>>+svn_error_t *
>>+svn_sqlite__column_properties(apr_hash_t **props,
>>+                              svn_sqlite__stmt_t *stmt,
>>+                              int column,
>>+                              apr_pool_t *result_pool,
>>+                              apr_pool_t *scratch_pool)
>
> Not related to this commit, but from my experience it's a good
> practice add boolean parameter to such database accessors to raise
> error on NULL value.
>
> I.e.:
> svn_sqlite__column_properties(apr_hash_t **props,
>                              svn_sqlite__stmt_t *stmt,
>                              int column,
>                              svn_boolean_t allow_null,
>                              apr_pool_t *result_pool,
>                              apr_pool_t *scratch_pool)
>
> I know that NOT NULL are enforcement by sqlite schema, but it's better
> to prevent dereference null pointer in our code. Sometimes schema
> changes while some of accessors does not updated.

The docstring for the above function:

/* Return the column as a hash of const char * => const svn_string_t *.
   If the column is null, then NULL will be stored into *PROPS. The
   results will be allocated in RESULT_POOL, and any temporary allocations
   will be made in SCRATCH_POOL. */

iow, the function says that a null column is NOT an error, but simply
returns NULL in *props. If the caller cares, then it can raise an
error by looking for that NULL pointer.

As far as "to prevent dereference [of a] null pointer" ... we pass
NULL pointers all the time. And this NULL pointer is part of the
interface contract. I think that callers will be responsible enough to
avoid errors.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1219727
Received on 2009-02-24 09:34:38 CET

This is an archived mail posted to the Subversion Dev mailing list.