[SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

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