"samdb_msg_add_string" cleanup

[Andrew Bartlett]

On Mon, 2010-11-29 at 12:47 +0100, Matthias Dieter Wallnöfer wrote:
> s4 developers,
> 
> we've three important calls which add strings to LDB messages: 
> "ldb_msg_add_string", "ldb_msg_add_steal_string" and "samdb_msg_add_string".
> They're not equivalent but I'm asking myself if like other "samdb_msg_*" 
> calls we should get rid of the last one. The only difference between the 
> first and the last one is the talloc'ed string value. Well, and in most 
> cases it's equivalent if it's talloc'ed or not (context allocations, 
> string literals as "somevalue"...).
> Thoughts?

It would seem to me that if we had:

int ldb_msg_add_dup_string(struct ldb_message *msg, 
		   	   const char *attr_name, const char *str)
{
	struct ldb_val val;

	val.length = strlen(str);
	if (val.length == 0) {

		/* allow empty strings as non-existent attributes */
		return LDB_SUCCESS;
	}
	val.data = talloc_strdup(msg, str);
	if (val.data) {
	        return LDB_ERR_OPERATIONAL;
	}

	return ldb_msg_add_steal_value(msg, attr_name, &val, NULL);
}

Then this would be equivalent in almost all cases, and an improvement in
talloc hierarchy.  We should consider if we want to add another LDB API
however, as it may be better just to change samdb_msg_add_string to have
this behaviour.  

However, even this would need to be handled carefully, and I'm thinking
that it might be better to leave cleanup patches such as this alone for
a little while, until we sort out the issues with the other recent
changes you have made. 

Please don't make a 'simple' translation to ldb_msg_add_string(), each
case does need to be looked at carefully. 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20101130/b9442944/attachment.pgp>