[PATCH] Fix bug #11177 - no talloc stackframe at ../source3/libsmb/clifsinfo.c:444, leaking memory
Volker Lendecke
Volker.Lendecke at SerNet.DE
Tue Apr 14 04:28:36 MDT 2015
On Tue, Apr 14, 2015 at 05:28:53PM +1200, Andrew Bartlett wrote:
> Do we still get that when each function has a talloc_stackframe(), and
> talloc_tos() is never called? That is all I'm really looking for.
Attached find a patchset that I just want to provide as an
example where I'm right now going for optimiziation: A lot
of talloc_tos() users can be removed with the variable
arrays in the second patch. C99 gives us this nice feature.
I am aware that this only covers a fraction of all
talloc_tos users, but it is a start.
The first patch is a simple conversion from tdb_fetch to
tdb_parse. This from my point of view is also simpler and
removes one user of the bad tdb_fetch API.
With -O3 on 64 bit this patchset shrinks the object code
size by 656 bytes, a value in itself.
Review & push appreciated!
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 55b2c2fefa2684135d35b6134294f52269f08b08 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 14 Apr 2015 10:06:55 +0000
Subject: [PATCH 1/2] winbind: Use tdb_parse_record in wcache_fetch_seqnum
This removes a malloc use
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/winbindd_cache.c | 57 +++++++++++++++++++++++----------------
1 file changed, 34 insertions(+), 23 deletions(-)
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 90270ba..1986aad 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -398,43 +398,54 @@ static bool wcache_server_down(struct winbindd_domain *domain)
return ret;
}
+struct wcache_seqnum_state {
+ uint32_t *seqnum;
+ uint32_t *last_seq_check;
+};
+
+static int wcache_seqnum_parser(TDB_DATA key, TDB_DATA data,
+ void *private_data)
+{
+ struct wcache_seqnum_state *state = private_data;
+
+ if (data.dsize != 8) {
+ DEBUG(10, ("wcache_fetch_seqnum: invalid data size %d\n",
+ (int)data.dsize));
+ return -1;
+ }
+
+ *state->seqnum = IVAL(data.dptr, 0);
+ *state->last_seq_check = IVAL(data.dptr, 4);
+ return 0;
+}
+
static bool wcache_fetch_seqnum(const char *domain_name, uint32_t *seqnum,
uint32_t *last_seq_check)
{
- char *key;
- TDB_DATA data;
+ struct wcache_seqnum_state state = {
+ .seqnum = seqnum, .last_seq_check = last_seq_check
+ };
+ char *keystr;
+ TDB_DATA key;
+ int ret;
if (wcache->tdb == NULL) {
DEBUG(10,("wcache_fetch_seqnum: tdb == NULL\n"));
return false;
}
- key = talloc_asprintf(talloc_tos(), "SEQNUM/%s", domain_name);
- if (key == NULL) {
+ keystr = talloc_asprintf(talloc_tos(), "SEQNUM/%s", domain_name);
+ if (keystr == NULL) {
DEBUG(10, ("talloc failed\n"));
return false;
}
+ key = string_term_tdb_data(keystr);
- data = tdb_fetch_bystring(wcache->tdb, key);
- TALLOC_FREE(key);
-
- if (data.dptr == NULL) {
- DEBUG(10, ("wcache_fetch_seqnum: %s not found\n",
- domain_name));
- return false;
- }
- if (data.dsize != 8) {
- DEBUG(10, ("wcache_fetch_seqnum: invalid data size %d\n",
- (int)data.dsize));
- SAFE_FREE(data.dptr);
- return false;
- }
-
- *seqnum = IVAL(data.dptr, 0);
- *last_seq_check = IVAL(data.dptr, 4);
- SAFE_FREE(data.dptr);
+ ret = tdb_parse_record(wcache->tdb, key, wcache_seqnum_parser,
+ &state);
+ TALLOC_FREE(keystr);
- return true;
+ return (ret == 0);
}
static NTSTATUS fetch_cache_seqnum( struct winbindd_domain *domain, time_t now )
--
1.9.1
From cac7a57840adee5b04b066464ffd6cc5b67ee75c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 14 Apr 2015 10:17:20 +0000
Subject: [PATCH 2/2] winbind: Avoid a few talloc_tos() in winbindd_cache.c
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/winbindd_cache.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 1986aad..def5fa0 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -425,8 +425,9 @@ static bool wcache_fetch_seqnum(const char *domain_name, uint32_t *seqnum,
struct wcache_seqnum_state state = {
.seqnum = seqnum, .last_seq_check = last_seq_check
};
- char *keystr;
- TDB_DATA key;
+ size_t len = strlen(domain_name);
+ char keystr[len+8];
+ TDB_DATA key = { .dptr = (uint8_t *)keystr, .dsize = sizeof(keystr) };
int ret;
if (wcache->tdb == NULL) {
@@ -434,17 +435,10 @@ static bool wcache_fetch_seqnum(const char *domain_name, uint32_t *seqnum,
return false;
}
- keystr = talloc_asprintf(talloc_tos(), "SEQNUM/%s", domain_name);
- if (keystr == NULL) {
- DEBUG(10, ("talloc failed\n"));
- return false;
- }
- key = string_term_tdb_data(keystr);
+ snprintf(keystr, sizeof(keystr), "SEQNUM/%s", domain_name);
ret = tdb_parse_record(wcache->tdb, key, wcache_seqnum_parser,
&state);
- TALLOC_FREE(keystr);
-
return (ret == 0);
}
@@ -478,7 +472,9 @@ static NTSTATUS fetch_cache_seqnum( struct winbindd_domain *domain, time_t now )
bool wcache_store_seqnum(const char *domain_name, uint32_t seqnum,
time_t last_seq_check)
{
- char *key_str;
+ size_t len = strlen(domain_name);
+ char keystr[len+8];
+ TDB_DATA key = { .dptr = (uint8_t *)keystr, .dsize = sizeof(keystr) };
uint8_t buf[8];
int ret;
@@ -487,22 +483,16 @@ bool wcache_store_seqnum(const char *domain_name, uint32_t seqnum,
return false;
}
- key_str = talloc_asprintf(talloc_tos(), "SEQNUM/%s", domain_name);
- if (key_str == NULL) {
- DEBUG(10, ("talloc_asprintf failed\n"));
- return false;
- }
+ snprintf(keystr, sizeof(keystr), "SEQNUM/%s", domain_name);
SIVAL(buf, 0, seqnum);
SIVAL(buf, 4, last_seq_check);
- ret = tdb_store_bystring(wcache->tdb, key_str,
- make_tdb_data(buf, sizeof(buf)), TDB_REPLACE);
- TALLOC_FREE(key_str);
+ ret = tdb_store(wcache->tdb, key, make_tdb_data(buf, sizeof(buf)),
+ TDB_REPLACE);
if (ret != 0) {
DEBUG(10, ("tdb_store_bystring failed: %s\n",
tdb_errorstr(wcache->tdb)));
- TALLOC_FREE(key_str);
return false;
}
--
1.9.1
More information about the samba-technical
mailing list