I just did a quick review of the deref control code. I thought we had already established this policy since the OpenLDAP 2.1 days: no more naked char *'s in new code. Always use struct bervals, never discard information you're going to need again later, and never recompute it if you already have it in hand. Every string sent thru the protocol is already accompanied by its length; it's stupid to call strlen() on any result we receive.
Seems that publishing this in RE24 was a bit premature, sorry I didn't catch this sooner.
On a related note, I'm still not so thrilled with the STRLENOF() macro. Most of the time I use sizeof("strconst") because I *know* there's a trailing NUL and I *want* it incorporated in the result...
-------- Original Message -------- Subject: (ITS#6233) print_deref assertion if no deref attributes returned Date: Tue, 28 Jul 2009 17:43:40 GMT From: rmeggins@redhat.com To: openldap-its@openldap.org
Full_Name: Rich Megginson Version: 2.4.17+ (current HEAD) OS: Fedora URL: ftp://ftp.openldap.org/incoming/ Submission from: (NULL) (76.113.59.19)
I am testing the LDAP Dereference feature specified here - http://www.openldap.org/devel/cvsweb.cgi/~checkout~/doc/drafts/draft-masarat...
I am getting an assertion in common.c at line 2077: 2074: ptr = lutil_strncopy( ptr, dr->derefVal.bv_val, dr->derefVal.bv_len ); 2075: *ptr++ = '\n'; 2076: *ptr++ = '\0'; 2077: assert( ptr <= buf + len );
tool_write_ldif( LDIF_PUT_COMMENT, NULL, buf, ptr - buf);
In my test, ptr is one greater than buf + len. It appears that len does not take into consideration the addition of the trailing \0 at line 2076. My test is looking for a dereference attribute which does not have any values, so that the attrVals in the control response is empty. If there are values to return in attrVals, the assertion is not triggered.
Howard Chu writes:
I just did a quick review of the deref control code. I thought we had already established this policy since the OpenLDAP 2.1 days: no more naked char *'s in new code. Always use struct bervals, (...)
Yes, I remember that for data structures at least. And exposed prototypes, or something like that.
Not for internals of a function, I hope. It's cumbersome (and harder for the compiler to optimize) to write bv->bv_val all the time instead of str.
On a related note, I'm still not so thrilled with the STRLENOF() macro. Most of the time I use sizeof("strconst") because I *know* there's a trailing NUL and I *want* it incorporated in the result...
Yes, in particular STRLENOF("") + 1 looks stupid. Though it's a bit more readable in a number of cases since it expresses the intent more directly.
One I really hate is AC_MEMCPY - partly because these capital letters are glaring at me for no good reason, but also because expands to memmove, not memcpy. Any reason not to use memcpy in cases where source and destination cannot overlap? That ought to be a bit faster too. (Might even go through the code and replace it for that reason.)
-
Hallvard B Furuseth -
Howard Chu