Re: (ITS#5578) sortvals error

[andrew.findlay@skills-1st.co.uk]

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
-- 
-----------------------------------------------------------------------
|                 From Andrew Findlay, Skills 1st Ltd                 |
| Consultant in large-scale systems, networks, and directory services |
|     http://www.skills-1st.co.uk/                +44 1628 782565     |
-----------------------------------------------------------------------