On Fri, Jun 27, 2008 at 03:17:10PM +0000, Andrew Findlay wrote:
Hmm: I now have a crasher in attr_valadd when adding a member to a group that already contains two members. Not sure yet whether I caused it with the minus-one-line fix above, or whether I have uncovered a new problem.
for ( j = a->a_numvals; j >= slot; j-- ) { a->a_vals[j+1] = a->a_vals[j]; <==================== Crash here if ( nvals ) a->a_nvals[j+1] = a->a_nvals[j]; }
... so I stepped through that in GDB. j did indeed start at 2 and decrement to 0, but then it kept going!
407 for ( j = a->a_numvals; j >= slot; j-- ) { (gdb) 408 a->a_vals[j+1] = a->a_vals[j]; (gdb) print j $17 = 1 (gdb) print a->a_vals[j] $18 = {bv_len = 97, bv_val = 0x847b680 "uniqueIdentifier=a3,ou=accounts,uniqueIdentifier=to-a,dc=orgs,...."} (gdb) n 409 if ( nvals ) (gdb) 410 a->a_nvals[j+1] = a->a_nvals[j]; (gdb) 407 for ( j = a->a_numvals; j >= slot; j-- ) { (gdb) 408 a->a_vals[j+1] = a->a_vals[j]; (gdb) print print a->a_vals[j] A syntax error in expression, near `a->a_vals[j]'. (gdb) print a->a_vals[j] $19 = {bv_len = 96, bv_val = 0x847b618 "uniqueIdentifier=ag1,ou=groups,uniqueIdentifier=to-a,dc=orgs,..."} (gdb) n slap_client_connect: URI=ldap://localhost:2389/ DN="cn=replicator,dc=accounts,...." ldap_sasl_bind_s failed (-1) do_syncrepl: rid=002 retrying (5 retries left) 409 if ( nvals ) (gdb) 410 a->a_nvals[j+1] = a->a_nvals[j]; (gdb) 407 for ( j = a->a_numvals; j >= slot; j-- ) { (gdb) 408 a->a_vals[j+1] = a->a_vals[j]; (gdb) print j $20 = -1 (gdb) n 409 if ( nvals ) (gdb) print slot $21 = 0 (gdb) n 410 a->a_nvals[j+1] = a->a_nvals[j]; (gdb) slap_client_connect: URI=ldap://localhost:2389/ DN="cn=replicator,dc=accounts,..." ldap_sasl_bind_s failed (-1) do_syncrepl: rid=002 retrying (4 retries left) 407 for ( j = a->a_numvals; j >= slot; j-- ) { (gdb) 408 a->a_vals[j+1] = a->a_vals[j]; (gdb) print j $22 = -2 (gdb) print j >= slot $23 = 1 (gdb) print slot $24 = 0
Curiouser and curiouser cried Alice...
(gdb) whatis j type = int (gdb) whatis slot type = unsigned int
Aha! Comparing signed and unsigned integers. Could be risky.
I don't do much C these days, and I don't see many explicit type-casts in OpenLDAP code. What would be a good-practice solution here?
Casting slot to long int seems to work for me so I have updated the patch: ftp://ftp.openldap.org/incoming/andrew.findlay-sortvals-20080627.patch
Andrew