Added lock backouts on fail.

When chaining together long lines of bloody "if" statements, which should
logically be separated, and one of them allocates memory, remember to
*free* it *WHETHER OR NOT THE IF STATEMENTS SUCCEEDED* !!!!
Yes I do consider this a bug in the coding style of Tridge, Rusty, Tim et al. :-).
I'm just pissed 'cos this took 4 hours to track down even with an insure error report
stating me in the face and also Ben Woodward looking over the code with me :-).
Jeremy.
(This used to be commit 506b5e34c3ba16768dbc82ba21044787de160c45)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 79c742e0a8f5ade8f1ebbb2f8bda89e1b25526ce..21121878651a47be49d3b5df7a24e517c17252a9 100644 (file)
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -3059,9 +3059,6 @@ void init_srv_share_info2(SH_INFO_2 *sh2,
                                char *net_name, uint32 type, char *remark,
                                uint32 perms, uint32 max_uses, uint32 num_uses,
                                char *path, char *passwd);
-void free_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr);
-void free_srv_q_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n);
-void free_srv_r_net_share_enum(SRV_R_NET_SHARE_ENUM *r_n);
 void init_srv_q_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n, 
                                char *srv_name, uint32 info_level,
                                uint32 preferred_len, ENUM_HND *hnd);

diff --git a/source3/rpc_client/cli_srvsvc.c b/source3/rpc_client/cli_srvsvc.c
index b6dc0aff4a37b879f7d796ea30b3176a1337c6b9..4a3aed6d159b0953abbc66f03912ea245672bf9b 100644 (file)
--- a/source3/rpc_client/cli_srvsvc.c
+++ b/source3/rpc_client/cli_srvsvc.c
@@ -243,7 +243,6 @@ BOOL do_srv_net_srv_share_enum(struct cli_state *cli,
                /* report error code */
                DEBUG(0,("SRV_R_NET_SHARE_ENUM: %s\n", get_nt_error_msg(r_o->status)));
                prs_mem_free(&rdata);
-               free_srv_r_net_share_enum(r_o);
                return False;
        }
 
@@ -252,7 +251,6 @@ BOOL do_srv_net_srv_share_enum(struct cli_state *cli,
                DEBUG(0,("SRV_R_NET_SHARE_ENUM: info class %d does not match request %d\n",
                        r_o->ctr.switch_value, switch_value));
                prs_mem_free(&rdata);
-               free_srv_r_net_share_enum(r_o);
                return False;
        }
 

diff --git a/source3/rpc_parse/parse_srv.c b/source3/rpc_parse/parse_srv.c
index be7401ffc5f76b22d135a1476ed4a6d7f24bd238..69cc1e7bda294290e746552a7d04da5a9fca44c7 100644 (file)
--- a/source3/rpc_parse/parse_srv.c
+++ b/source3/rpc_parse/parse_srv.c
@@ -291,7 +291,7 @@ static BOOL srv_io_srv_share_ctr(char *desc, SRV_SHARE_INFO_CTR *ctr, prs_struct
                int i;
 
                if (UNMARSHALLING(ps)) {
-                       if (!(info1 = malloc(num_entries * sizeof(SRV_SHARE_INFO_1))))
+                       if (!(info1 = prs_alloc_mem(ps, num_entries * sizeof(SRV_SHARE_INFO_1))))
                                return False;
                        memset(info1, '\0', num_entries * sizeof(SRV_SHARE_INFO_1));
                        ctr->share.info1 = info1;
@@ -317,7 +317,7 @@ static BOOL srv_io_srv_share_ctr(char *desc, SRV_SHARE_INFO_CTR *ctr, prs_struct
                int i;
 
                if (UNMARSHALLING(ps)) {
-                       if (!(info2 = malloc(num_entries * sizeof(SRV_SHARE_INFO_2))))
+                       if (!(info2 = prs_alloc_mem(ps,num_entries * sizeof(SRV_SHARE_INFO_2))))
                                return False;
                        memset(info2, '\0', num_entries * sizeof(SRV_SHARE_INFO_2));
                        ctr->share.info2 = info2;
@@ -345,43 +345,6 @@ static BOOL srv_io_srv_share_ctr(char *desc, SRV_SHARE_INFO_CTR *ctr, prs_struct
        return True;
 }
 
-/*******************************************************************
- Frees a SRV_SHARE_INFO_CTR structure.
-********************************************************************/
-
-void free_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr)
-{
-       if(!ctr)
-               return;
-       if(ctr->share.info)
-               free(ctr->share.info);
-       memset(ctr, '\0', sizeof(SRV_SHARE_INFO_CTR));
-}
-
-/*******************************************************************
- Frees a SRV_Q_NET_SHARE_ENUM structure.
-********************************************************************/
-
-void free_srv_q_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n)
-{
-       if(!q_n)
-               return;
-       free_srv_share_info_ctr(&q_n->ctr);
-       memset(q_n, '\0', sizeof(SRV_Q_NET_SHARE_ENUM));
-}
-
-/*******************************************************************
- Frees a SRV_R_NET_SHARE_ENUM structure.
-********************************************************************/
-
-void free_srv_r_net_share_enum(SRV_R_NET_SHARE_ENUM *r_n)
-{
-       if(!r_n)
-               return;
-       free_srv_share_info_ctr(&r_n->ctr);
-       memset(r_n, '\0', sizeof(SRV_R_NET_SHARE_ENUM));
-}
-
 /*******************************************************************
  Inits a SRV_Q_NET_SHARE_ENUM structure.
 ********************************************************************/

diff --git a/source3/rpc_server/srv_srvsvc.c b/source3/rpc_server/srv_srvsvc.c
index 715a681a299f6d7a0b97673379a4da237795e02c..52ae54fd94a1f488abdf518b5171b217256c2001 100644 (file)
--- a/source3/rpc_server/srv_srvsvc.c
+++ b/source3/rpc_server/srv_srvsvc.c
@@ -109,7 +109,7 @@ static void init_srv_share_info_1005(SRV_SHARE_INFO_1005* sh1005, int snum)
  Fill in a share info structure.
  ********************************************************************/
 
-static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr,
+static BOOL init_srv_share_info_ctr(TALLOC_CTX *ctx, SRV_SHARE_INFO_CTR *ctr,
               uint32 info_level, uint32 *resume_hnd, uint32 *total_entries)
 {
        int num_entries = 0;
@@ -142,7 +142,7 @@ static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr,
                SRV_SHARE_INFO_1 *info1;
                int i = 0;
 
-               info1 = malloc(num_entries * sizeof(SRV_SHARE_INFO_1));
+               info1 = talloc(ctx, num_entries * sizeof(SRV_SHARE_INFO_1));
 
                for (snum = *resume_hnd; snum < num_services; snum++) {
                        if (lp_browseable(snum) && lp_snum_ok(snum)) {
@@ -159,7 +159,7 @@ static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr,
                SRV_SHARE_INFO_2 *info2;
                int i = 0;
 
-               info2 = malloc(num_entries * sizeof(SRV_SHARE_INFO_2));
+               info2 = talloc(ctx, num_entries * sizeof(SRV_SHARE_INFO_2));
 
                for (snum = *resume_hnd; snum < num_services; snum++) {
                        if (lp_browseable(snum) && lp_snum_ok(snum)) {
@@ -183,12 +183,12 @@ static BOOL init_srv_share_info_ctr(SRV_SHARE_INFO_CTR *ctr,
  Inits a SRV_R_NET_SHARE_ENUM structure.
 ********************************************************************/
 
-static void init_srv_r_net_share_enum(SRV_R_NET_SHARE_ENUM *r_n,
+static void init_srv_r_net_share_enum(TALLOC_CTX *ctx, SRV_R_NET_SHARE_ENUM *r_n,
                                      uint32 info_level, uint32 resume_hnd)  
 {
        DEBUG(5,("init_srv_r_net_share_enum: %d\n", __LINE__));
 
-       if (init_srv_share_info_ctr(&r_n->ctr, info_level,
+       if (init_srv_share_info_ctr(ctx, &r_n->ctr, info_level,
                                    &resume_hnd, &r_n->total_entries)) {
                r_n->status = 0x0;
        } else {
@@ -207,21 +207,25 @@ static BOOL srv_reply_net_share_enum(SRV_Q_NET_SHARE_ENUM *q_n,
 {
        SRV_R_NET_SHARE_ENUM r_n;
        BOOL ret;
+       TALLOC_CTX *ctx = talloc_init();
 
        DEBUG(5,("srv_net_share_enum: %d\n", __LINE__));
 
+       if (!ctx) {
+               DEBUG(0,("srv_reply_net_share_enum: talloc_init failed.\n"));
+               return False;
+       }
+
        /* Create the list of shares for the response. */
-       init_srv_r_net_share_enum(&r_n,
+       init_srv_r_net_share_enum(ctx, &r_n,
                                q_n->ctr.info_level,
                                get_enum_hnd(&q_n->enum_hnd));
 
        /* store the response in the SMB stream */
        ret = srv_io_r_net_share_enum("", &r_n, rdata, 0);
 
-       /* Free the memory used by the response. */
-       free_srv_r_net_share_enum(&r_n);
-
        DEBUG(5,("srv_net_share_enum: %d\n", __LINE__));
+       talloc_destroy(ctx);
 
        return ret;
 }
@@ -286,9 +290,6 @@ static BOOL srv_reply_net_share_get_info(SRV_Q_NET_SHARE_GET_INFO *q_n,
        /* store the response in the SMB stream */
        ret = srv_io_r_net_share_get_info("", &r_n, rdata, 0);
 
-       /* Free the memory used by the response. */
-       free_srv_r_net_share_get_info(&r_n);
-
        DEBUG(5,("srv_net_share_get_info: %d\n", __LINE__));
 
        return ret;
@@ -1024,9 +1025,6 @@ static BOOL api_srv_net_share_enum(pipes_struct *p)
 
        ret = srv_reply_net_share_enum(&q_n, rdata);
 
-       /* Free any data allocated in the unmarshalling. */
-       free_srv_q_net_share_enum(&q_n);
-
        return ret;
 }
 
@@ -1049,9 +1047,6 @@ static BOOL api_srv_net_share_get_info(pipes_struct *p)
 
        ret = srv_reply_net_share_get_info(&q_n, rdata);
 
-       /* Free any data allocated in the unmarshalling. */
-       free_srv_q_net_share_get_info(&q_n);
-
        return ret;
 }
 

diff --git a/source3/tdb/tdb.c b/source3/tdb/tdb.c
index ede38f27f9f2d79dd0d90e53297c9d42697466bd..afc87b7da0735d17ebbad9b6efaca0a341706ae1 100644 (file)
--- a/source3/tdb/tdb.c
+++ b/source3/tdb/tdb.c
@@ -922,11 +922,13 @@ TDB_DATA tdb_nextkey(TDB_CONTEXT *tdb, TDB_DATA oldkey)
                                            rec.key_len))
                    || memcmp(k, oldkey.dptr, oldkey.dsize) != 0) {
                        /* No, it wasn't: unlock it and start from scratch */
-                       free(k);
                        unlock_record(tdb, tdb->travlocks.off);
                        tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK);
                        tdb->travlocks.off = 0;
                }
+
+               if (k)
+                       free(k);
        }
 
        if (!tdb->travlocks.off) {
@@ -1180,7 +1182,19 @@ int tdb_lockall(TDB_CONTEXT *tdb)
        /* There are no locks on read-only dbs */
        if (tdb->read_only) return TDB_ERRCODE(TDB_ERR_LOCK, -1);
        if (tdb->lockedkeys) return TDB_ERRCODE(TDB_ERR_NOLOCK, -1);
-       for (i = 0; i < tdb->header.hash_size; i++) tdb_lock(tdb, i, F_WRLCK);
+       for (i = 0; i < tdb->header.hash_size; i++) 
+               if (tdb_lock(tdb, i, F_WRLCK))
+                       break;
+
+       /* If error, release locks we have... */
+       if (i < tdb->header.hash_size) {
+               u32 j;
+
+               for ( j = 0; j < i; j++)
+                       tdb_unlock(tdb, j, F_WRLCK);
+               return TDB_ERRCODE(TDB_ERR_NOLOCK, -1);
+       }
+
        return 0;
 }
 void tdb_unlockall(TDB_CONTEXT *tdb)
@@ -1211,7 +1225,18 @@ int tdb_lockkeys(TDB_CONTEXT *tdb, u32 number, TDB_DATA keys[])
                tdb->lockedkeys[j+1] = hash;
        }
        /* Finally, lock in order */
-       for (i = 0; i < number; i++) tdb_lock(tdb, i, F_WRLCK);
+       for (i = 0; i < number; i++)
+               if (tdb_lock(tdb, i, F_WRLCK))
+                       break;
+
+       /* If error, release locks we have... */
+       if (i < number) {
+               for ( j = 0; j < i; j++)
+                       tdb_unlock(tdb, j, F_WRLCK);
+               free(tdb->lockedkeys);
+               tdb->lockedkeys = NULL;
+               return TDB_ERRCODE(TDB_ERR_NOLOCK, -1);
+       }
        return 0;
 }