[SCM] Samba Shared Repository - branch master updated
Simo Sorce
idra at samba.org
Fri Jun 18 07:58:39 MDT 2010
The branch, master has been updated
via 2e8c563... Second part of fix converting prs_XX struct and functions to talloc. Remove unneeded prs_mem_free calls.
via f4e7d9d... Convert the prs_XXX struct and functions to use talloc instead of malloc. Passes valgrind and make tests for client and server. Second version of this patch after splitting up at Simo's request. Patch to follow will delete extraneous prs_mem_free() calls.
from 3fa3804... unittest: Change the name of the base ldb test so that it can be easily called separatly
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 2e8c5639618fa4c6efb461ad4e6b51c84da8bd10
Author: Jeremy Allison <jra at samba.org>
Date: Thu Jun 17 15:51:19 2010 -0700
Second part of fix converting prs_XX struct and functions to talloc. Remove unneeded prs_mem_free calls.
Jeremy.
Signed-off-by: Simo Sorce <idra at samba.org>
commit f4e7d9d38ebf209be6c040f4ef98dd8111735187
Author: Jeremy Allison <jra at samba.org>
Date: Thu Jun 17 15:35:07 2010 -0700
Convert the prs_XXX struct and functions to use talloc instead of malloc. Passes valgrind and make tests for client and server. Second version of this patch after splitting up at Simo's request. Patch to follow will delete extraneous prs_mem_free() calls.
Jeremy.
Signed-off-by: Simo Sorce <idra at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/include/ntdomain.h | 4 +-
source3/rpc_client/cli_pipe.c | 71 ++++------------------------
source3/rpc_client/ndr.c | 2 -
source3/rpc_parse/parse_prs.c | 52 ++++++++++++++------
source3/rpc_server/rpc_ncacn_np_internal.c | 4 --
source3/rpc_server/srv_pipe_hnd.c | 57 +++++++++++-----------
6 files changed, 78 insertions(+), 112 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/include/ntdomain.h b/source3/include/ntdomain.h
index 213cb9f..25bef47 100644
--- a/source3/include/ntdomain.h
+++ b/source3/include/ntdomain.h
@@ -42,7 +42,9 @@ typedef struct _prs_struct {
uint32 data_offset; /* Current working offset into data. */
uint32 buffer_size; /* Current allocated size of the buffer. */
uint32 grow_size; /* size requested via prs_grow() calls */
- char *data_p; /* The buffer itself. */
+ /* The buffer itself. If "is_dynamic" is true this
+ * MUST BE TALLOC'ed off mem_ctx. */
+ char *data_p;
TALLOC_CTX *mem_ctx; /* When unmarshalling, use this.... */
} prs_struct;
diff --git a/source3/rpc_client/cli_pipe.c b/source3/rpc_client/cli_pipe.c
index e248133..69f8fb5 100644
--- a/source3/rpc_client/cli_pipe.c
+++ b/source3/rpc_client/cli_pipe.c
@@ -1373,13 +1373,6 @@ struct rpc_api_pipe_state {
uint32_t incoming_pdu_offset;
};
-static int rpc_api_pipe_state_destructor(struct rpc_api_pipe_state *state)
-{
- prs_mem_free(&state->incoming_frag);
- prs_mem_free(&state->incoming_pdu);
- return 0;
-}
-
static void rpc_api_pipe_trans_done(struct tevent_req *subreq);
static void rpc_api_pipe_got_pdu(struct tevent_req *subreq);
@@ -1409,8 +1402,6 @@ static struct tevent_req *rpc_api_pipe_send(TALLOC_CTX *mem_ctx,
/* Make incoming_pdu dynamic with no memory. */
prs_give_memory(&state->incoming_pdu, NULL, 0, true);
- talloc_set_destructor(state, rpc_api_pipe_state_destructor);
-
/*
* Ensure we're not sending too much.
*/
@@ -1453,7 +1444,6 @@ static void rpc_api_pipe_trans_done(struct tevent_req *subreq)
NTSTATUS status;
uint8_t *rdata = NULL;
uint32_t rdata_len = 0;
- char *rdata_copy;
status = cli_api_pipe_recv(subreq, state, &rdata, &rdata_len);
TALLOC_FREE(subreq);
@@ -1471,16 +1461,11 @@ static void rpc_api_pipe_trans_done(struct tevent_req *subreq)
}
/*
- * Give the memory received from cli_trans as dynamic to the current
- * pdu. Duplicating it sucks, but prs_struct doesn't know about talloc
- * :-(
+ * This is equivalent to a talloc_steal - gives rdata to
+ * the prs_struct state->incoming_frag.
*/
- rdata_copy = (char *)memdup(rdata, rdata_len);
- TALLOC_FREE(rdata);
- if (tevent_req_nomem(rdata_copy, req)) {
- return;
- }
- prs_give_memory(&state->incoming_frag, rdata_copy, rdata_len, true);
+ prs_give_memory(&state->incoming_frag, (char *)rdata, rdata_len, true);
+ rdata = NULL;
/* Ensure we have enough data for a pdu. */
subreq = get_complete_frag_send(state, state->ev, state->cli,
@@ -1597,9 +1582,10 @@ static NTSTATUS rpc_api_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
reply_pdu->mem_ctx = mem_ctx;
/*
- * Prevent state->incoming_pdu from being freed in
- * rpc_api_pipe_state_destructor()
+ * Prevent state->incoming_pdu from being freed
+ * when state is freed.
*/
+ talloc_steal(mem_ctx, prs_data_p(reply_pdu));
prs_init_empty(&state->incoming_pdu, state, UNMARSHALL);
return NT_STATUS_OK;
@@ -1638,7 +1624,6 @@ static NTSTATUS create_krb5_auth_bind_req( struct rpc_pipe_client *cli,
error_message(ret) ));
data_blob_free(&tkt);
- prs_mem_free(auth_data);
return NT_STATUS_INVALID_PARAMETER;
}
@@ -1650,7 +1635,6 @@ static NTSTATUS create_krb5_auth_bind_req( struct rpc_pipe_client *cli,
/* Auth len in the rpc header doesn't include auth_header. */
if (!prs_copy_data_in(auth_data, (char *)tkt_wrapped.data, tkt_wrapped.length)) {
data_blob_free(&tkt_wrapped);
- prs_mem_free(auth_data);
return NT_STATUS_NO_MEMORY;
}
@@ -1688,7 +1672,6 @@ static NTSTATUS create_spnego_ntlmssp_auth_rpc_bind_req( struct rpc_pipe_client
if (!NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
data_blob_free(&request);
- prs_mem_free(auth_data);
return nt_status;
}
@@ -1700,7 +1683,6 @@ static NTSTATUS create_spnego_ntlmssp_auth_rpc_bind_req( struct rpc_pipe_client
/* Auth len in the rpc header doesn't include auth_header. */
if (!prs_copy_data_in(auth_data, (char *)spnego_msg.data, spnego_msg.length)) {
data_blob_free(&spnego_msg);
- prs_mem_free(auth_data);
return NT_STATUS_NO_MEMORY;
}
@@ -1734,14 +1716,12 @@ static NTSTATUS create_ntlmssp_auth_rpc_bind_req( struct rpc_pipe_client *cli,
if (!NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
data_blob_free(&request);
- prs_mem_free(auth_data);
return nt_status;
}
/* Auth len in the rpc header doesn't include auth_header. */
if (!prs_copy_data_in(auth_data, (char *)request.data, request.length)) {
data_blob_free(&request);
- prs_mem_free(auth_data);
return NT_STATUS_NO_MEMORY;
}
@@ -1791,7 +1771,6 @@ static NTSTATUS create_schannel_auth_rpc_bind_req( struct rpc_pipe_client *cli,
(ndr_push_flags_fn_t)ndr_push_NL_AUTH_MESSAGE);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
DEBUG(0,("Failed to marshall NL_AUTH_MESSAGE.\n"));
- prs_mem_free(auth_data);
return ndr_map_error2ntstatus(ndr_err);
}
@@ -1801,7 +1780,6 @@ static NTSTATUS create_schannel_auth_rpc_bind_req( struct rpc_pipe_client *cli,
if (!prs_copy_data_in(auth_data, (const char *)blob.data, blob.length))
{
- prs_mem_free(auth_data);
return NT_STATUS_NO_MEMORY;
}
@@ -1914,7 +1892,6 @@ static NTSTATUS create_rpc_bind_req(struct rpc_pipe_client *cli,
case PIPE_AUTH_TYPE_SCHANNEL:
ret = create_schannel_auth_rpc_bind_req(cli, auth_level, &hdr_auth, &auth_info);
if (!NT_STATUS_IS_OK(ret)) {
- prs_mem_free(&auth_info);
return ret;
}
break;
@@ -1922,7 +1899,6 @@ static NTSTATUS create_rpc_bind_req(struct rpc_pipe_client *cli,
case PIPE_AUTH_TYPE_NTLMSSP:
ret = create_ntlmssp_auth_rpc_bind_req(cli, auth_level, &hdr_auth, &auth_info);
if (!NT_STATUS_IS_OK(ret)) {
- prs_mem_free(&auth_info);
return ret;
}
break;
@@ -1930,7 +1906,6 @@ static NTSTATUS create_rpc_bind_req(struct rpc_pipe_client *cli,
case PIPE_AUTH_TYPE_SPNEGO_NTLMSSP:
ret = create_spnego_ntlmssp_auth_rpc_bind_req(cli, auth_level, &hdr_auth, &auth_info);
if (!NT_STATUS_IS_OK(ret)) {
- prs_mem_free(&auth_info);
return ret;
}
break;
@@ -1938,7 +1913,6 @@ static NTSTATUS create_rpc_bind_req(struct rpc_pipe_client *cli,
case PIPE_AUTH_TYPE_KRB5:
ret = create_krb5_auth_bind_req(cli, auth_level, &hdr_auth, &auth_info);
if (!NT_STATUS_IS_OK(ret)) {
- prs_mem_free(&auth_info);
return ret;
}
break;
@@ -1959,7 +1933,6 @@ static NTSTATUS create_rpc_bind_req(struct rpc_pipe_client *cli,
&hdr_auth,
&auth_info);
- prs_mem_free(&auth_info);
return ret;
}
@@ -2207,13 +2180,6 @@ struct rpc_api_pipe_req_state {
prs_struct reply_pdu;
};
-static int rpc_api_pipe_req_state_destructor(struct rpc_api_pipe_req_state *s)
-{
- prs_mem_free(&s->outgoing_frag);
- prs_mem_free(&s->reply_pdu);
- return 0;
-}
-
static void rpc_api_pipe_req_write_done(struct tevent_req *subreq);
static void rpc_api_pipe_req_done(struct tevent_req *subreq);
static NTSTATUS prepare_next_frag(struct rpc_api_pipe_req_state *state,
@@ -2256,8 +2222,6 @@ struct tevent_req *rpc_api_pipe_req_send(TALLOC_CTX *mem_ctx,
goto fail;
}
- talloc_set_destructor(state, rpc_api_pipe_req_state_destructor);
-
status = prepare_next_frag(state, &is_last_frag);
if (!NT_STATUS_IS_OK(status)) {
goto post_status;
@@ -2458,9 +2422,10 @@ NTSTATUS rpc_api_pipe_req_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
reply_pdu->mem_ctx = mem_ctx;
/*
- * Prevent state->req_pdu from being freed in
- * rpc_api_pipe_req_state_destructor()
+ * Prevent state->req_pdu from being freed
+ * when state is freed.
*/
+ talloc_steal(mem_ctx, prs_data_p(reply_pdu));
prs_init_empty(&state->reply_pdu, state, UNMARSHALL);
return NT_STATUS_OK;
@@ -2627,7 +2592,6 @@ static NTSTATUS create_rpc_alter_context(uint32 rpc_call_id,
if (pauth_blob->length) {
if (!prs_copy_data_in(&auth_info, (const char *)pauth_blob->data, pauth_blob->length)) {
- prs_mem_free(&auth_info);
return NT_STATUS_NO_MEMORY;
}
}
@@ -2639,7 +2603,6 @@ static NTSTATUS create_rpc_alter_context(uint32 rpc_call_id,
transfer,
&hdr_auth,
&auth_info);
- prs_mem_free(&auth_info);
return ret;
}
@@ -2654,12 +2617,6 @@ struct rpc_pipe_bind_state {
uint32_t rpc_call_id;
};
-static int rpc_pipe_bind_state_destructor(struct rpc_pipe_bind_state *state)
-{
- prs_mem_free(&state->rpc_out);
- return 0;
-}
-
static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq);
static NTSTATUS rpc_finish_auth3_bind_send(struct tevent_req *req,
struct rpc_pipe_bind_state *state,
@@ -2696,7 +2653,6 @@ struct tevent_req *rpc_pipe_bind_send(TALLOC_CTX *mem_ctx,
state->rpc_call_id = get_rpc_call_id();
prs_init_empty(&state->rpc_out, state, MARSHALL);
- talloc_set_destructor(state, rpc_pipe_bind_state_destructor);
cli->auth = talloc_move(cli, &auth);
@@ -2752,7 +2708,6 @@ static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq)
/* Unmarshall the RPC header */
if (!smb_io_rpc_hdr("hdr", &hdr, &reply_pdu, 0)) {
DEBUG(0, ("rpc_pipe_bind: failed to unmarshall RPC_HDR.\n"));
- prs_mem_free(&reply_pdu);
tevent_req_nterror(req, NT_STATUS_BUFFER_TOO_SMALL);
return;
}
@@ -2760,14 +2715,12 @@ static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq)
if (!smb_io_rpc_hdr_ba("", &hdr_ba, &reply_pdu, 0)) {
DEBUG(0, ("rpc_pipe_bind: Failed to unmarshall "
"RPC_HDR_BA.\n"));
- prs_mem_free(&reply_pdu);
tevent_req_nterror(req, NT_STATUS_BUFFER_TOO_SMALL);
return;
}
if (!check_bind_response(&hdr_ba, &state->cli->transfer_syntax)) {
DEBUG(2, ("rpc_pipe_bind: check_bind_response failed.\n"));
- prs_mem_free(&reply_pdu);
tevent_req_nterror(req, NT_STATUS_BUFFER_TOO_SMALL);
return;
}
@@ -2784,7 +2737,6 @@ static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq)
case PIPE_AUTH_TYPE_NONE:
case PIPE_AUTH_TYPE_SCHANNEL:
/* Bind complete. */
- prs_mem_free(&reply_pdu);
tevent_req_done(req);
break;
@@ -2792,7 +2744,6 @@ static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq)
/* Need to send AUTH3 packet - no reply. */
status = rpc_finish_auth3_bind_send(req, state, &hdr,
&reply_pdu);
- prs_mem_free(&reply_pdu);
if (!NT_STATUS_IS_OK(status)) {
tevent_req_nterror(req, status);
}
@@ -2802,7 +2753,6 @@ static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq)
/* Need to send alter context request and reply. */
status = rpc_finish_spnego_ntlmssp_bind_send(req, state, &hdr,
&reply_pdu);
- prs_mem_free(&reply_pdu);
if (!NT_STATUS_IS_OK(status)) {
tevent_req_nterror(req, status);
}
@@ -2814,7 +2764,6 @@ static void rpc_pipe_bind_step_one_done(struct tevent_req *subreq)
default:
DEBUG(0,("cli_finish_bind_auth: unknown auth type %u\n",
(unsigned int)state->cli->auth->auth_type));
- prs_mem_free(&reply_pdu);
tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
}
}
diff --git a/source3/rpc_client/ndr.c b/source3/rpc_client/ndr.c
index bbd7806..8e03f2e 100644
--- a/source3/rpc_client/ndr.c
+++ b/source3/rpc_client/ndr.c
@@ -103,7 +103,6 @@ static void cli_do_rpc_ndr_done(struct tevent_req *subreq)
status = rpc_api_pipe_req_recv(subreq, state, &state->r_ps);
TALLOC_FREE(subreq);
- prs_mem_free(&state->q_ps);
if (!NT_STATUS_IS_OK(status)) {
tevent_req_nterror(req, status);
return;
@@ -126,7 +125,6 @@ NTSTATUS cli_do_rpc_ndr_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx)
}
ret = prs_data_blob(&state->r_ps, &blob, talloc_tos());
- prs_mem_free(&state->r_ps);
if (!ret) {
return NT_STATUS_NO_MEMORY;
}
diff --git a/source3/rpc_parse/parse_prs.c b/source3/rpc_parse/parse_prs.c
index 42e6ce1..af1cc3d 100644
--- a/source3/rpc_parse/parse_prs.c
+++ b/source3/rpc_parse/parse_prs.c
@@ -93,7 +93,7 @@ void prs_debug(prs_struct *ps, int depth, const char *desc, const char *fn_name)
* Initialise an expandable parse structure.
*
* @param size Initial buffer size. If >0, a new buffer will be
- * created with malloc().
+ * created with talloc().
*
* @return False if allocation fails, otherwise True.
**/
@@ -112,11 +112,11 @@ bool prs_init(prs_struct *ps, uint32 size, TALLOC_CTX *ctx, bool io)
if (size != 0) {
ps->buffer_size = size;
- if((ps->data_p = (char *)SMB_MALLOC((size_t)size)) == NULL) {
- DEBUG(0,("prs_init: malloc fail for %u bytes.\n", (unsigned int)size));
+ ps->data_p = talloc_zero_size(ps->mem_ctx, size);
+ if(ps->data_p == NULL) {
+ DEBUG(0,("prs_init: talloc fail for %u bytes.\n", (unsigned int)size));
return False;
}
- memset(ps->data_p, '\0', (size_t)size);
ps->is_dynamic = True; /* We own this memory. */
} else if (MARSHALLING(ps)) {
/* If size is zero and we're marshalling we should allocate memory on demand. */
@@ -130,14 +130,18 @@ bool prs_init(prs_struct *ps, uint32 size, TALLOC_CTX *ctx, bool io)
Delete the memory in a parse structure - if we own it.
NOTE: Contrary to the somewhat confusing naming, this function is not
- intended for freeing memory allocated by prs_alloc_mem(). That memory
- is attached to the talloc context given by ps->mem_ctx.
+ intended for freeing memory allocated by prs_alloc_mem().
+ That memory is also attached to the talloc context given by
+ ps->mem_ctx, but is only freed when that talloc context is
+ freed. prs_mem_free() is used to delete "dynamic" memory
+ allocated in marshalling/unmarshalling.
********************************************************************/
void prs_mem_free(prs_struct *ps)
{
- if(ps->is_dynamic)
- SAFE_FREE(ps->data_p);
+ if(ps->is_dynamic) {
+ TALLOC_FREE(ps->data_p);
+ }
ps->is_dynamic = False;
ps->buffer_size = 0;
ps->data_offset = 0;
@@ -184,11 +188,17 @@ TALLOC_CTX *prs_get_mem_context(prs_struct *ps)
/*******************************************************************
Hand some already allocated memory to a prs_struct.
+ If "is_dynamic" is true, this is a talloc_steal.
+ If "is_dynamic" is false, just make ps->data_p a pointer to
+ the unwritable memory.
********************************************************************/
void prs_give_memory(prs_struct *ps, char *buf, uint32 size, bool is_dynamic)
{
ps->is_dynamic = is_dynamic;
+ if (ps->is_dynamic && buf) {
+ talloc_steal(ps->mem_ctx, buf);
+ }
ps->data_p = buf;
ps->buffer_size = size;
}
@@ -207,14 +217,16 @@ bool prs_set_buffer_size(prs_struct *ps, uint32 newsize)
/* newsize == 0 acts as a free and set pointer to NULL */
if (newsize == 0) {
- SAFE_FREE(ps->data_p);
+ TALLOC_FREE(ps->data_p);
} else {
- ps->data_p = (char *)SMB_REALLOC(ps->data_p, newsize);
+ ps->data_p = talloc_realloc(ps->mem_ctx,
+ ps->data_p,
+ char,
+ newsize);
if (ps->data_p == NULL) {
DEBUG(0,("prs_set_buffer_size: Realloc failure for size %u.\n",
(unsigned int)newsize));
- DEBUG(0,("prs_set_buffer_size: Reason %s\n",strerror(errno)));
return False;
}
}
@@ -261,11 +273,11 @@ bool prs_grow(prs_struct *ps, uint32 extra_space)
*/
new_size = MAX(128, extra_space);
- if((ps->data_p = (char *)SMB_MALLOC(new_size)) == NULL) {
- DEBUG(0,("prs_grow: Malloc failure for size %u.\n", (unsigned int)new_size));
+ ps->data_p = (char *)talloc_zero_size(ps->mem_ctx, new_size);
+ if(ps->data_p == NULL) {
+ DEBUG(0,("prs_grow: talloc failure for size %u.\n", (unsigned int)new_size));
return False;
}
- memset(ps->data_p, '\0', (size_t)new_size );
} else {
/*
* If the current buffer size is bigger than the space needed,
@@ -276,7 +288,11 @@ bool prs_grow(prs_struct *ps, uint32 extra_space)
new_size = MAX(ps->buffer_size*2,
ps->buffer_size + extra_space + 64);
- if ((ps->data_p = (char *)SMB_REALLOC(ps->data_p, new_size)) == NULL) {
+ ps->data_p = talloc_realloc(ps->mem_ctx,
+ ps->data_p,
+ char,
+ new_size);
+ if (ps->data_p == NULL) {
DEBUG(0,("prs_grow: Realloc failure for size %u.\n",
(unsigned int)new_size));
return False;
@@ -305,7 +321,11 @@ bool prs_force_grow(prs_struct *ps, uint32 extra_space)
return False;
}
- if((ps->data_p = (char *)SMB_REALLOC(ps->data_p, new_size)) == NULL) {
+ ps->data_p = (char *)talloc_realloc(ps->mem_ctx,
+ ps->data_p,
+ char,
+ new_size);
+ if(ps->data_p == NULL) {
DEBUG(0,("prs_force_grow: Realloc failure for size %u.\n",
(unsigned int)new_size));
return False;
diff --git a/source3/rpc_server/rpc_ncacn_np_internal.c b/source3/rpc_server/rpc_ncacn_np_internal.c
index e8e8f90..f9317b9 100644
--- a/source3/rpc_server/rpc_ncacn_np_internal.c
+++ b/source3/rpc_server/rpc_ncacn_np_internal.c
@@ -91,10 +91,6 @@ static int close_internal_rpc_pipe_hnd(struct pipes_struct *p)
return False;
}
- prs_mem_free(&p->out_data.frag);
- prs_mem_free(&p->out_data.rdata);
- prs_mem_free(&p->in_data.data);
-
if (p->auth.auth_data_free_func) {
(*p->auth.auth_data_free_func)(&p->auth);
}
diff --git a/source3/rpc_server/srv_pipe_hnd.c b/source3/rpc_server/srv_pipe_hnd.c
index 975f5b8..a77b9ea 100644
--- a/source3/rpc_server/srv_pipe_hnd.c
+++ b/source3/rpc_server/srv_pipe_hnd.c
@@ -237,24 +237,29 @@ static ssize_t unmarshall_rpc_header(pipes_struct *p)
}
/****************************************************************************
- Call this to free any talloc'ed memory. Do this before and after processing
- a complete PDU.
+ Call this to free any talloc'ed memory. Do this after processing
+ a complete incoming and outgoing request (multiple incoming/outgoing
+ PDU's).
****************************************************************************/
static void free_pipe_context(pipes_struct *p)
{
- if (p->mem_ctx) {
- DEBUG(3, ("free_pipe_context: "
- "destroying talloc pool of size %lu\n",
--
Samba Shared Repository
More information about the samba-cvs
mailing list