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