Re: [gnome-db] [PATCH] Fix some memory leaks



On Sun, Mar 16, 2008 at 8:00 PM, Phil Longstaff <plongstaff rogers com> wrote:
> Murray Cumming wrote:
>  > On Sun, 2008-03-16 at 13:20 -0400, Phil Longstaff wrote:
>  >> The attached patch fixes some memory leaks.  In certain cases, the
>  >> string returned from g_object_get() or xmlNodeGetContent() was not freed.
>  >
>  > Thanks. Is this change meant to be in this patch? It seems unrelated.
>
>  Yes, this is unrelated.  It is a previous patch that I sent in which was
>  accidentally included.
>
>
>  > In general, it is helpful if you can create an svn patch, also patching
>  > the ChangeLog. And patches are less likely to be forgotten if they are
>  > attached to a bugzilla bug.
>  >
>
>  I am writing a libgda backend for gnucash.  I am trying to use
>  pre-packaged versions of libgda, so in this case, I downloaded the 3.0.2
>   tarball and built it.  Perhaps using the 3.x svn branch would be
>  better.  Since this is an external dependency, I want it to be as
>  controlled as possible in my environment.
>
>
>  > You might want to try creating a similar patch for svn trunk too.
>  >
>
>  Can't you apply this to the 3.x branch and then merge the change into
>  svn trunk?
>
>  BTW, once I found a problem with g_object_set() and xmlNodeGetContent(),
>  I grepped all of the source in libgda and checked their usage.  I didn't
>  look in any subdirs and did not look in any providers.
>
>  Do you have a test program that you can run with valgrind to look for
>  memory leaks as part of your release process?
>
>  Oh, one other thing.  There was another thing.
>  gda_server_provider_create_operation() begins with:
>
>   OpReq **op_req_table = NULL;
>
>   if (! op_req_table) {
>     op_req_table = g_new0 (OpReq *, GDA_SERVER_OPERATION_NB);
>
>     op_req_table [GDA_SERVER_OPERATION_CREATE_DB] = op_req_CREATE_DB;
>     op_req_table [GDA_SERVER_OPERATION_DROP_DB] = op_req_DROP_DB;
>
>     op_req_table [GDA_SERVER_OPERATION_CREATE_TABLE] = op_req_CREATE_TABLE;
>     op_req_table [GDA_SERVER_OPERATION_DROP_TABLE] = op_req_DROP_TABLE;
>     op_req_table [GDA_SERVER_OPERATION_RENAME_TABLE] = op_req_RENAME_TABLE;
>
>     op_req_table [GDA_SERVER_OPERATION_ADD_COLUMN] = op_req_ADD_COLUMN;
>     op_req_table [GDA_SERVER_OPERATION_DROP_COLUMN] = op_req_DROP_COLUMN;
>
>     op_req_table [GDA_SERVER_OPERATION_CREATE_INDEX] = op_req_CREATE_INDEX;
>     op_req_table [GDA_SERVER_OPERATION_DROP_INDEX] = op_req_DROP_INDEX;
>   }
>
>   g_return_val_if_fail (GDA_IS_SERVER_PROVIDER (provider), NULL);
>
>  This will always leak.  I assume that what is needed is to make
>  op_req_table static, but I did not include this in the patch and did not
>  scan for other instances of this kind of problem.  In this case, it
>  won't leak once per call.

This has been corrected in the V3 branch, and will soon be in the V4
(made it static). Thanks,

Vivien


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]