Hello list,
While trying to figure out why openchangesim require so much memory I
found that I had long lasting ldb objects and cli_credential.
But making profile to be talloced we can use profile as a talloc context
for ldb requests and for cli_credential thus allowing desallocation of
memory once the profile is not needed anymore.
I have also I patch to free the session in duplicate profile and another
to have correctly initialized structures so that valgrind doesn't complain.
Any comments ?
Matthieu
>From ff51d200e17dc834bc31564443e75569c11efdf2 Mon Sep 17 00:00:00 2001
From: Matthieu Patou <[email protected]>
Date: Tue, 13 Mar 2012 11:54:55 -0700
Subject: [PATCH 1/4] Free the session in duplicateprofile
---
libmapi/IProfAdmin.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/libmapi/IProfAdmin.c b/libmapi/IProfAdmin.c
index b651eba..75dfc6b 100644
--- a/libmapi/IProfAdmin.c
+++ b/libmapi/IProfAdmin.c
@@ -1139,6 +1139,8 @@ _PUBLIC_ enum MAPISTATUS DuplicateProfile(struct mapi_context *mapi_ctx,
set_profile_attribute(mapi_ctx, profile_dst, *SRowSet, index, PR_EMAIL_ADDRESS, "EmailAddress");
mapi_profile_delete_string_attr(mapi_ctx, profile_dst, "EmailAddress", oldEmailAddress);
MAPIFreeBuffer(SRowSet);
+ DLIST_REMOVE(mapi_ctx->session, session);
+ MAPIFreeBuffer(session);
}
/* Change ProxyAddress */
--
1.7.5.4
>From f8db2472f16b8f7b1fc37ec9ba1bf46d785598ea Mon Sep 17 00:00:00 2001
From: Matthieu Patou <[email protected]>
Date: Thu, 15 Mar 2012 16:38:48 -0700
Subject: [PATCH 2/4] Cleanly initialize structures for mapi_response
---
libmapi/IUnknown.c | 2 ++
ndr_mapi.c | 4 +++-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/libmapi/IUnknown.c b/libmapi/IUnknown.c
index 44c1eb6..85988b9 100644
--- a/libmapi/IUnknown.c
+++ b/libmapi/IUnknown.c
@@ -122,6 +122,8 @@ _PUBLIC_ enum MAPISTATUS Release(mapi_object_t *obj)
enum MAPISTATUS retval;
uint8_t logon_id = 0;
+ mapi_response = 0;
+
/* Sanity checks */
session = mapi_object_get_session(obj);
OPENCHANGE_RETVAL_IF(!session, MAPI_E_INVALID_PARAMETER, NULL);
diff --git a/ndr_mapi.c b/ndr_mapi.c
index 2448178..d1e661d 100644
--- a/ndr_mapi.c
+++ b/ndr_mapi.c
@@ -633,7 +633,7 @@ enum ndr_err_code ndr_pull_mapi_response(struct ndr_pull *ndr, int ndr_flags, st
/* If length equals length field then skipping subcontext */
if (r->length > sizeof (uint16_t)) {
_mem_save_mapi_repl_0 = NDR_PULL_GET_MEM_CTX(ndr);
- r->mapi_repl = talloc_array(_mem_save_mapi_repl_0, struct EcDoRpc_MAPI_REPL, 2);
+ r->mapi_repl = talloc_zero_array(_mem_save_mapi_repl_0, struct EcDoRpc_MAPI_REPL, 2);
NDR_CHECK(ndr_pull_subcontext_start(ndr, &_ndr_mapi_repl, 0, r->length - 2));
for (cntr_mapi_repl_0 = 0; _ndr_mapi_repl->offset < _ndr_mapi_repl->data_size - 2; cntr_mapi_repl_0++) {
NDR_CHECK(ndr_pull_EcDoRpc_MAPI_REPL(_ndr_mapi_repl, NDR_SCALARS, &r->mapi_repl[cntr_mapi_repl_0]));
@@ -642,6 +642,8 @@ enum ndr_err_code ndr_pull_mapi_response(struct ndr_pull *ndr, int ndr_flags, st
r->mapi_repl[cntr_mapi_repl_0].opnum = 0;
NDR_CHECK(ndr_pull_subcontext_end(ndr, _ndr_mapi_repl, 4, -1));
talloc_free(_ndr_mapi_repl);
+ } else {
+ r->mapi_repl = NULL;
}
_mem_save_handles_0 = NDR_PULL_GET_MEM_CTX(ndr);
--
1.7.5.4
>From 63ebb4a4573f51a3492b602a5a2301eba177573e Mon Sep 17 00:00:00 2001
From: Matthieu Patou <[email protected]>
Date: Fri, 16 Mar 2012 17:38:50 -0700
Subject: [PATCH 3/4] Move to talloced profile so that we can free ressources
more easily
---
libmapi/IProfAdmin.c | 54 ++++++++++++++++++++++++++++++-------------------
utils/mapiprofile.c | 30 ++++++++++++++-------------
2 files changed, 49 insertions(+), 35 deletions(-)
diff --git a/libmapi/IProfAdmin.c b/libmapi/IProfAdmin.c
index 75dfc6b..35d8afe 100644
--- a/libmapi/IProfAdmin.c
+++ b/libmapi/IProfAdmin.c
@@ -57,30 +57,39 @@ static enum MAPISTATUS ldb_load_profile(TALLOC_CTX *mem_ctx,
if (ret != LDB_SUCCESS) return MAPI_E_NOT_FOUND;
/* profile not found */
- if (!res->count) return MAPI_E_NOT_FOUND;
+ if (!res->count) {
+ talloc_free(res);
+ return MAPI_E_NOT_FOUND;
+ }
+
/* more than one profile */
- if (res->count > 1) return MAPI_E_COLLISION;
+ if (res->count > 1) {
+ talloc_free(res);
+ return MAPI_E_COLLISION;
+ }
/* fills in profile with query result */
msg = res->msgs[0];
- profile->username = ldb_msg_find_attr_as_string(msg, "username", NULL);
- profile->password = password ? password : ldb_msg_find_attr_as_string(msg, "password", NULL);
- profile->workstation = ldb_msg_find_attr_as_string(msg, "workstation", NULL);
- profile->realm = ldb_msg_find_attr_as_string(msg, "realm", NULL);
- profile->domain = ldb_msg_find_attr_as_string(msg, "domain", NULL);
- profile->mailbox = ldb_msg_find_attr_as_string(msg, "EmailAddress", NULL);
- profile->homemdb = ldb_msg_find_attr_as_string(msg, "HomeMDB", NULL);
- profile->localaddr = ldb_msg_find_attr_as_string(msg, "localaddress", NULL);
- profile->server = ldb_msg_find_attr_as_string(msg, "binding", NULL);
+ profile->username = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "username", NULL));
+ profile->password = password ? password : talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "password", NULL));
+ profile->workstation = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "workstation", NULL));
+ profile->realm = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "realm", NULL));
+ profile->domain = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "domain", NULL));
+ profile->mailbox = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "EmailAddress", NULL));
+ profile->homemdb = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "HomeMDB", NULL));
+ profile->localaddr = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "localaddress", NULL));
+ profile->server = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "binding", NULL));
profile->seal = ldb_msg_find_attr_as_bool(msg, "seal", false);
- profile->org = ldb_msg_find_attr_as_string(msg, "Organization", NULL);
- profile->ou = ldb_msg_find_attr_as_string(msg, "OrganizationUnit", NULL);
+ profile->org = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "Organization", NULL));
+ profile->ou = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "OrganizationUnit", NULL));
profile->codepage = ldb_msg_find_attr_as_int(msg, "codepage", 0);
profile->language = ldb_msg_find_attr_as_int(msg, "language", 0);
profile->method = ldb_msg_find_attr_as_int(msg, "method", 0);
profile->exchange_version = ldb_msg_find_attr_as_int(msg, "exchange_version", 0);
- profile->kerberos = ldb_msg_find_attr_as_string(msg, "kerberos", NULL);
+ profile->kerberos = talloc_steal(profile, ldb_msg_find_attr_as_string(msg, "kerberos", NULL));
+
+ talloc_free(res);
return MAPI_E_SUCCESS;
}
@@ -1022,7 +1031,7 @@ _PUBLIC_ enum MAPISTATUS DuplicateProfile(struct mapi_context *mapi_ctx,
char *username_src = NULL;
char *ProxyAddress = NULL;
char *oldEmailAddress = NULL;
- struct mapi_profile profile;
+ struct mapi_profile *profile;
char **attr_tmp = NULL;
char *tmp = NULL;
char *attr;
@@ -1034,27 +1043,29 @@ _PUBLIC_ enum MAPISTATUS DuplicateProfile(struct mapi_context *mapi_ctx,
OPENCHANGE_RETVAL_IF(!profile_dst, MAPI_E_INVALID_PARAMETER, NULL);
OPENCHANGE_RETVAL_IF(!username, MAPI_E_INVALID_PARAMETER, NULL);
+ mem_ctx = talloc_named(mapi_ctx->mem_ctx, 0, "DuplicateProfile");
+ profile = talloc(mem_ctx, struct mapi_profile);
+
/* Step 1. Copy the profile */
retval = CopyProfile(mapi_ctx, profile_src, profile_dst);
OPENCHANGE_RETVAL_IF(retval, retval, NULL);
/* retrieve username, EmailAddress and ProxyAddress */
- retval = OpenProfile(mapi_ctx, &profile, profile_src, NULL);
+ retval = OpenProfile(mapi_ctx, profile, profile_src, NULL);
OPENCHANGE_RETVAL_IF(retval, MAPI_E_NOT_FOUND, NULL);
- mem_ctx = talloc_named(mapi_ctx->mem_ctx, 0, "DuplicateProfile");
- retval = GetProfileAttr(&profile, "username", &count, &attr_tmp);
+ retval = GetProfileAttr(profile, "username", &count, &attr_tmp);
OPENCHANGE_RETVAL_IF(retval || !count, MAPI_E_NOT_FOUND, mem_ctx);
username_src = talloc_strdup(mem_ctx, attr_tmp[0]);
talloc_free(attr_tmp[0]);
- retval = GetProfileAttr(&profile, "EmailAddress", &count, &attr_tmp);
+ retval = GetProfileAttr(profile, "EmailAddress", &count, &attr_tmp);
OPENCHANGE_RETVAL_IF(retval, MAPI_E_NOT_FOUND, mem_ctx);
oldEmailAddress = talloc_strdup(mem_ctx, attr_tmp[0]);
talloc_free(attr_tmp[0]);
- retval = GetProfileAttr(&profile, "ProxyAddress", &count, &attr_tmp);
+ retval = GetProfileAttr(profile, "ProxyAddress", &count, &attr_tmp);
OPENCHANGE_RETVAL_IF(retval, MAPI_E_NOT_FOUND, mem_ctx);
ProxyAddress = talloc_strdup(mem_ctx, attr_tmp[0]);
talloc_free(attr_tmp[0]);
@@ -1072,7 +1083,7 @@ _PUBLIC_ enum MAPISTATUS DuplicateProfile(struct mapi_context *mapi_ctx,
char *password;
struct mapi_session *session = NULL;
- retval = GetProfileAttr(&profile, "password", &count, &attr_tmp);
+ retval = GetProfileAttr(profile, "password", &count, &attr_tmp);
OPENCHANGE_RETVAL_IF(retval || !count, MAPI_E_NOT_FOUND, mem_ctx);
password = talloc_strdup(mem_ctx, attr_tmp[0]);
talloc_free(attr_tmp[0]);
@@ -1159,6 +1170,7 @@ _PUBLIC_ enum MAPISTATUS DuplicateProfile(struct mapi_context *mapi_ctx,
mapi_profile_modify_string_attr(mapi_ctx, profile_dst, "DisplayName", username);
mapi_profile_modify_string_attr(mapi_ctx, profile_dst, "Account", username);
+ talloc_free(profile);
talloc_free(mem_ctx);
return MAPI_E_SUCCESS;
diff --git a/utils/mapiprofile.c b/utils/mapiprofile.c
index b3331eb..b85aa4f 100644
--- a/utils/mapiprofile.c
+++ b/utils/mapiprofile.c
@@ -402,11 +402,12 @@ static void mapiprofile_dump(struct mapi_context *mapi_ctx, const char *profdb,
{
TALLOC_CTX *mem_ctx;
enum MAPISTATUS retval;
- struct mapi_profile profile;
+ struct mapi_profile *profile;
char *profname;
char *exchange_version = NULL;
mem_ctx = talloc_named(mapi_ctx->mem_ctx, 0, "mapiprofile_dump");
+ profile = talloc(mem_ctx, struct mapi_profile);
if (!opt_profname) {
if ((retval = GetDefaultProfile(mapi_ctx, &profname)) != MAPI_E_SUCCESS) {
@@ -418,7 +419,7 @@ static void mapiprofile_dump(struct mapi_context *mapi_ctx, const char *profdb,
profname = talloc_strdup(mem_ctx, (const char *)opt_profname);
}
- retval = OpenProfile(mapi_ctx, &profile, profname, NULL);
+ retval = OpenProfile(mapi_ctx, profile, profname, NULL);
talloc_free(profname);
if (retval && (retval != MAPI_E_INVALID_PARAMETER)) {
@@ -427,7 +428,7 @@ static void mapiprofile_dump(struct mapi_context *mapi_ctx, const char *profdb,
exit (1);
}
- switch (profile.exchange_version) {
+ switch (profile->exchange_version) {
case 0x0:
exchange_version = talloc_strdup(mem_ctx, "exchange 2000");
break;
@@ -442,15 +443,15 @@ static void mapiprofile_dump(struct mapi_context *mapi_ctx, const char *profdb,
goto end;
}
- printf("Profile: %s\n", profile.profname);
+ printf("Profile: %s\n", profile->profname);
printf("\texchange server == %s\n", exchange_version);
- printf("\tencryption == %s\n", (profile.seal == true) ? "yes" : "no");
- printf("\tusername == %s\n", profile.username);
- printf("\tpassword == %s\n", profile.password);
- printf("\tmailbox == %s\n", profile.mailbox);
- printf("\tworkstation == %s\n", profile.workstation);
- printf("\tdomain == %s\n", profile.domain);
- printf("\tserver == %s\n", profile.server);
+ printf("\tencryption == %s\n", (profile->seal == true) ? "yes" : "no");
+ printf("\tusername == %s\n", profile->username);
+ printf("\tpassword == %s\n", profile->password);
+ printf("\tmailbox == %s\n", profile->mailbox);
+ printf("\tworkstation == %s\n", profile->workstation);
+ printf("\tdomain == %s\n", profile->domain);
+ printf("\tserver == %s\n", profile->server);
end:
talloc_free(mem_ctx);
@@ -461,13 +462,14 @@ static void mapiprofile_attribute(struct mapi_context *mapi_ctx, const char *pro
{
TALLOC_CTX *mem_ctx;
enum MAPISTATUS retval;
- struct mapi_profile profile;
+ struct mapi_profile *profile;
char *profname = NULL;
char **value = NULL;
unsigned int count = 0;
unsigned int i;
mem_ctx = talloc_named(mapi_ctx->mem_ctx, 0, "mapiprofile_attribute");
+ profile = talloc(mem_ctx, struct mapi_profile);
if (!opt_profname) {
if ((retval = GetDefaultProfile(mapi_ctx, &profname)) != MAPI_E_SUCCESS) {
@@ -478,7 +480,7 @@ static void mapiprofile_attribute(struct mapi_context *mapi_ctx, const char *pro
profname = talloc_strdup(mem_ctx, (const char *)opt_profname);
}
- retval = OpenProfile(mapi_ctx, &profile, profname, NULL);
+ retval = OpenProfile(mapi_ctx, profile, profname, NULL);
if (retval && (retval != MAPI_E_INVALID_PARAMETER)) {
mapi_errstr("OpenProfile", retval);
talloc_free(profname);
@@ -486,7 +488,7 @@ static void mapiprofile_attribute(struct mapi_context *mapi_ctx, const char *pro
exit (1);
}
- if ((retval = GetProfileAttr(&profile, attribute, &count, &value))) {
+ if ((retval = GetProfileAttr(profile, attribute, &count, &value))) {
mapi_errstr("ProfileGetAttr", retval);
talloc_free(profname);
talloc_free(mem_ctx);
--
1.7.5.4
>From 6d0a674141ce1a713b1cc6aff269617e52defacb Mon Sep 17 00:00:00 2001
From: Matthieu Patou <[email protected]>
Date: Sun, 18 Mar 2012 19:25:06 -0700
Subject: [PATCH 4/4] Use the profile as a talloc context for credentials,
allow ressources to be freed
---
libmapi/IProfAdmin.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libmapi/IProfAdmin.c b/libmapi/IProfAdmin.c
index 35d8afe..35967fd 100644
--- a/libmapi/IProfAdmin.c
+++ b/libmapi/IProfAdmin.c
@@ -761,7 +761,7 @@ _PUBLIC_ enum MAPISTATUS LoadProfile(struct mapi_context *mapi_ctx,
mem_ctx = mapi_ctx->mem_ctx;
- profile->credentials = cli_credentials_init(mem_ctx);
+ profile->credentials = cli_credentials_init(profile);
OPENCHANGE_RETVAL_IF(!profile->credentials, MAPI_E_NOT_ENOUGH_RESOURCES, NULL);
cli_credentials_guess(profile->credentials, mapi_ctx->lp_ctx);
if (profile->workstation && *(profile->workstation)) {
--
1.7.5.4
_______________________________________________
devel mailing list
[email protected]
http://mailman.openchange.org/listinfo/devel
