Hi Matthias,

On 03/06/10 20:01, Matthias Dieter Wallnöfer wrote:
diff --git a/source4/lib/registry/ldb.c b/source4/lib/registry/ldb.c
index 68639f5..12722c9 100644
--- a/source4/lib/registry/ldb.c
+++ b/source4/lib/registry/ldb.c
@@ -105,7 +107,8 @@ static struct ldb_message *reg_ldb_pack_value(struct 
ldb_context *ctx,
        switch (type) {
        case REG_SZ:
        case REG_EXPAND_SZ:
-               if (data.data[0] != '\0') {
+               if ((data.length>  0)&&  (data.data != NULL)
+               &&  (data.data[0] != '\0')) {
^^^ Why this overparanoid check ? If data.length is invalid we've got bigger fish to fry.
                        convert_string_talloc(mem_ctx, CH_UTF16, CH_UTF8,
                                                   (void *)data.data,
                                                   data.length,
@@ -116,23 +119,27 @@ static struct ldb_message *reg_ldb_pack_value(struct 
ldb_context *ctx,
                }
                break;

-       case REG_BINARY:
-               if (data.length>  0)
-                       ldb_msg_add_value(msg, "data",&data, NULL);
-               else
+       case REG_DWORD:
+               if ((data.length>  0)&&  (data.data != NULL)) {
^^^ Again, why this overparanoid check?
+                       ldb_msg_add_string(msg, "data",
+                                          talloc_asprintf(mem_ctx, "0x%x",
+                                                          IVAL(data.data, 0)));
+               } else {
                        ldb_msg_add_empty(msg, "data", LDB_FLAG_MOD_DELETE, 
NULL);
+               }
                break;

-       case REG_DWORD:
-               ldb_msg_add_string(msg, "data",
-                                  talloc_asprintf(mem_ctx, "0x%x",
-                                                  IVAL(data.data, 0)));
-               break;
+       case REG_BINARY:
        default:
-               ldb_msg_add_value(msg, "data",&data, NULL);
+               if ((data.length>  0)&&  (data.data != NULL)
+               &&  (data.data[0] != '\0')) {
+                       ldb_msg_add_value(msg, "data",&data, NULL);
+               } else {
+                       ldb_msg_add_empty(msg, "data", LDB_FLAG_MOD_DELETE, 
NULL);
+               }
+               break;
        }
The delete doesn't seem right here - an empty value is not the same as no value at all.

@@ -709,14 +718,27 @@ static WERROR ldb_set_value(struct hive_key *parent,
                }
        }

-       ret = ldb_add(kd->ldb, msg);
-       if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) {
-               int i;
-               for (i = 0; i<  msg->num_elements; i++) {
-                       if (msg->elements[i].flags != LDB_FLAG_MOD_DELETE)
-                               msg->elements[i].flags = LDB_FLAG_MOD_REPLACE;
+       /* Try first a "modify" and if this doesn't work do try an "add" */
+       for (i = 0; i<  msg->num_elements; i++) {
+               if (msg->elements[i].flags != LDB_FLAG_MOD_DELETE) {
+                       msg->elements[i].flags = LDB_FLAG_MOD_REPLACE;
                }
-               ret = ldb_modify(kd->ldb, msg);
+       }
+       ret = ldb_modify(kd->ldb, msg);
+       if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+               i = 0;
+               while (i<  msg->num_elements) {
+                       if (msg->elements[i].flags == LDB_FLAG_MOD_DELETE) {
+                               ldb_msg_remove_element(msg,&msg->elements[i]);
+                       } else {
+                               ++i;
+                       }
+               }
+               ret = ldb_add(kd->ldb, msg);
+       }
+       if (ret == LDB_ERR_NO_SUCH_ATTRIBUTE) {
+               /* ignore this ->  the value didn't exist and also now doesn't 
*/
+               ret = LDB_SUCCESS;
        }

        if (ret != LDB_SUCCESS) {
Can you explain the reasoning behind this change?

It doesn't seem quite right to ignore LDB_ERR_NO_SUCH_ATTRIBUTE. Why would that ever apply?

Cheers,

Jelmer

Reply via email to