svn commit: samba r21563 - in branches: SAMBA_3_0/source/rpc_server
SAMBA_3_0_25/source/rpc_server
vlendec at samba.org
vlendec at samba.org
Tue Feb 27 17:21:23 GMT 2007
Author: vlendec
Date: 2007-02-27 17:21:21 +0000 (Tue, 27 Feb 2007)
New Revision: 21563
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=21563
Log:
Fix a memleak: We only need dispinfo structs for "our" and for the builtin
domain. Without this patch we leaked a DISPINFO for the (NULL) domain per
samr_connect*() call.
Volker
Modified:
branches/SAMBA_3_0/source/rpc_server/srv_samr_nt.c
branches/SAMBA_3_0_25/source/rpc_server/srv_samr_nt.c
Changeset:
Modified: branches/SAMBA_3_0/source/rpc_server/srv_samr_nt.c
===================================================================
--- branches/SAMBA_3_0/source/rpc_server/srv_samr_nt.c 2007-02-27 17:17:16 UTC (rev 21562)
+++ branches/SAMBA_3_0/source/rpc_server/srv_samr_nt.c 2007-02-27 17:21:21 UTC (rev 21563)
@@ -46,8 +46,6 @@
#define DISP_INFO_CACHE_TIMEOUT 10
typedef struct disp_info {
- struct disp_info *next, *prev;
- TALLOC_CTX *mem_ctx;
DOM_SID sid; /* identify which domain this is. */
BOOL builtin_domain; /* Quick flag to check if this is the builtin domain. */
struct pdb_search *users; /* querydispinfo 1 and 4 */
@@ -65,8 +63,6 @@
/* We keep a static list of these by SID as modern clients close down
all resources between each request in a complete enumeration. */
-static DISP_INFO *disp_info_list;
-
struct samr_info {
/* for use by the \PIPE\samr policy */
DOM_SID sid;
@@ -254,49 +250,59 @@
Fetch or create a dispinfo struct.
********************************************************************/
-static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid, const char *sid_str)
+static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid)
{
- TALLOC_CTX *mem_ctx;
- DISP_INFO *dpi;
+ /*
+ * We do a static cache for DISP_INFO's here. Explanation can be found
+ * in Jeremy's checkin message to r11793:
+ *
+ * Fix the SAMR cache so it works across completely insane
+ * client behaviour (ie.:
+ * open pipe/open SAMR handle/enumerate 0 - 1024
+ * close SAMR handle, close pipe.
+ * open pipe/open SAMR handle/enumerate 1024 - 2048...
+ * close SAMR handle, close pipe.
+ * And on ad-nausium. Amazing.... probably object-oriented
+ * client side programming in action yet again.
+ * This change should *massively* improve performance when
+ * enumerating users from an LDAP database.
+ * Jeremy.
+ *
+ * "Our" and the builtin domain are the only ones where we ever
+ * enumerate stuff, so just cache 2 entries.
+ */
+ static struct disp_info builtin_dispinfo;
+ static struct disp_info domain_dispinfo;
+
/* There are two cases to consider here:
1) The SID is a domain SID and we look for an equality match, or
2) This is an account SID and so we return the DISP_INFO* for our
domain */
- if ( psid && sid_check_is_in_our_domain( psid ) ) {
- DEBUG(10,("get_samr_dispinfo_by_sid: Replacing %s with our domain SID\n",
- sid_str));
- psid = get_global_sam_sid();
+ if (psid == NULL) {
+ return NULL;
}
- for (dpi = disp_info_list; dpi; dpi = dpi->next) {
- if (sid_equal(psid, &dpi->sid)) {
- return dpi;
- }
+ if (sid_check_is_builtin(psid) || sid_check_is_in_builtin(psid)) {
+ /*
+ * Necessary only once, but it does not really hurt.
+ */
+ sid_copy(&builtin_dispinfo.sid, &global_sid_Builtin);
+
+ return &builtin_dispinfo;
}
+
+ if (sid_check_is_domain(psid) || sid_check_is_in_our_domain(psid)) {
+ /*
+ * Necessary only once, but it does not really hurt.
+ */
+ sid_copy(&domain_dispinfo.sid, get_global_sam_sid());
- /* This struct is never free'd - I'm using talloc so we
- can get a list out of smbd using smbcontrol. There will
- be one of these per SID we're authorative for. JRA. */
-
- mem_ctx = talloc_init("DISP_INFO for domain sid %s", sid_str);
-
- if ((dpi = TALLOC_ZERO_P(mem_ctx, DISP_INFO)) == NULL)
- return NULL;
-
- dpi->mem_ctx = mem_ctx;
-
- if (psid) {
- sid_copy( &dpi->sid, psid);
- dpi->builtin_domain = sid_check_is_builtin(psid);
- } else {
- dpi->builtin_domain = False;
+ return &domain_dispinfo;
}
- DLIST_ADD(disp_info_list, dpi);
-
- return dpi;
+ return NULL;
}
/*******************************************************************
@@ -330,13 +336,8 @@
}
info->mem_ctx = mem_ctx;
- info->disp_info = get_samr_dispinfo_by_sid(psid, sid_str);
+ info->disp_info = get_samr_dispinfo_by_sid(psid);
- if (!info->disp_info) {
- talloc_destroy(mem_ctx);
- return NULL;
- }
-
return info;
}
Modified: branches/SAMBA_3_0_25/source/rpc_server/srv_samr_nt.c
===================================================================
--- branches/SAMBA_3_0_25/source/rpc_server/srv_samr_nt.c 2007-02-27 17:17:16 UTC (rev 21562)
+++ branches/SAMBA_3_0_25/source/rpc_server/srv_samr_nt.c 2007-02-27 17:21:21 UTC (rev 21563)
@@ -46,8 +46,6 @@
#define DISP_INFO_CACHE_TIMEOUT 10
typedef struct disp_info {
- struct disp_info *next, *prev;
- TALLOC_CTX *mem_ctx;
DOM_SID sid; /* identify which domain this is. */
BOOL builtin_domain; /* Quick flag to check if this is the builtin domain. */
struct pdb_search *users; /* querydispinfo 1 and 4 */
@@ -65,8 +63,6 @@
/* We keep a static list of these by SID as modern clients close down
all resources between each request in a complete enumeration. */
-static DISP_INFO *disp_info_list;
-
struct samr_info {
/* for use by the \PIPE\samr policy */
DOM_SID sid;
@@ -254,49 +250,59 @@
Fetch or create a dispinfo struct.
********************************************************************/
-static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid, const char *sid_str)
+static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid)
{
- TALLOC_CTX *mem_ctx;
- DISP_INFO *dpi;
+ /*
+ * We do a static cache for DISP_INFO's here. Explanation can be found
+ * in Jeremy's checkin message to r11793:
+ *
+ * Fix the SAMR cache so it works across completely insane
+ * client behaviour (ie.:
+ * open pipe/open SAMR handle/enumerate 0 - 1024
+ * close SAMR handle, close pipe.
+ * open pipe/open SAMR handle/enumerate 1024 - 2048...
+ * close SAMR handle, close pipe.
+ * And on ad-nausium. Amazing.... probably object-oriented
+ * client side programming in action yet again.
+ * This change should *massively* improve performance when
+ * enumerating users from an LDAP database.
+ * Jeremy.
+ *
+ * "Our" and the builtin domain are the only ones where we ever
+ * enumerate stuff, so just cache 2 entries.
+ */
+ static struct disp_info builtin_dispinfo;
+ static struct disp_info domain_dispinfo;
+
/* There are two cases to consider here:
1) The SID is a domain SID and we look for an equality match, or
2) This is an account SID and so we return the DISP_INFO* for our
domain */
- if ( psid && sid_check_is_in_our_domain( psid ) ) {
- DEBUG(10,("get_samr_dispinfo_by_sid: Replacing %s with our domain SID\n",
- sid_str));
- psid = get_global_sam_sid();
+ if (psid == NULL) {
+ return NULL;
}
- for (dpi = disp_info_list; dpi; dpi = dpi->next) {
- if (sid_equal(psid, &dpi->sid)) {
- return dpi;
- }
+ if (sid_check_is_builtin(psid) || sid_check_is_in_builtin(psid)) {
+ /*
+ * Necessary only once, but it does not really hurt.
+ */
+ sid_copy(&builtin_dispinfo.sid, &global_sid_Builtin);
+
+ return &builtin_dispinfo;
}
+
+ if (sid_check_is_domain(psid) || sid_check_is_in_our_domain(psid)) {
+ /*
+ * Necessary only once, but it does not really hurt.
+ */
+ sid_copy(&domain_dispinfo.sid, get_global_sam_sid());
- /* This struct is never free'd - I'm using talloc so we
- can get a list out of smbd using smbcontrol. There will
- be one of these per SID we're authorative for. JRA. */
-
- mem_ctx = talloc_init("DISP_INFO for domain sid %s", sid_str);
-
- if ((dpi = TALLOC_ZERO_P(mem_ctx, DISP_INFO)) == NULL)
- return NULL;
-
- dpi->mem_ctx = mem_ctx;
-
- if (psid) {
- sid_copy( &dpi->sid, psid);
- dpi->builtin_domain = sid_check_is_builtin(psid);
- } else {
- dpi->builtin_domain = False;
+ return &domain_dispinfo;
}
- DLIST_ADD(disp_info_list, dpi);
-
- return dpi;
+ return NULL;
}
/*******************************************************************
@@ -330,13 +336,8 @@
}
info->mem_ctx = mem_ctx;
- info->disp_info = get_samr_dispinfo_by_sid(psid, sid_str);
+ info->disp_info = get_samr_dispinfo_by_sid(psid);
- if (!info->disp_info) {
- talloc_destroy(mem_ctx);
- return NULL;
- }
-
return info;
}
More information about the samba-cvs
mailing list