From 862f01a8b7e1057f4a228c0705ee7e42ed1a506a Mon Sep 17 00:00:00 2001 From: Kurt Zeilenga Date: Mon, 23 Jan 2006 20:09:55 +0000 Subject: [PATCH] From HEAD: + Fixed slapd-hdb livelock issue (ITS#4360) + Fixed slapd-bdb/hdb auto-recovery issue (ITS#4361) + Fixed slapd-bdb/hdb BDB 4.4 compatibility issues (ITS#4362) + Fixed slapo-ppolicy modify issue (ITS#4355) --- CHANGES | 4 + servers/slapd/alock.c | 44 ++++-- servers/slapd/alock.h | 5 + servers/slapd/back-bdb/init.c | 251 ++++++++++--------------------- servers/slapd/overlays/ppolicy.c | 10 +- servers/slapd/schemaparse.c | 100 +++++++----- 6 files changed, 187 insertions(+), 227 deletions(-) diff --git a/CHANGES b/CHANGES index a3ab2c296f..6b98d6d20d 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,10 @@ OpenLDAP 2.3 Change Log OpenLDAP 2.3.19 Engineering Fixed libldap disable DH key exchange with DH params (ITS#4354) Fixed libldap_r pool destroy hang (ITS#4349) + Fixed slapd-hdb livelock issue (ITS#4360) + Fixed slapd-bdb/hdb auto-recovery issue (ITS#4361) + Fixed slapd-bdb/hdb BDB 4.4 compatibility issues (ITS#4362) + Fixed slapo-ppolicy modify issue (ITS#4355) Build environment Fixed slaptools when --disable-debug (ITS#4351) Fixed slapd(8) solaris select(2) issue (ITS#4357) diff --git a/servers/slapd/alock.c b/servers/slapd/alock.c index abec648e55..b0bb33789f 100644 --- a/servers/slapd/alock.c +++ b/servers/slapd/alock.c @@ -290,7 +290,7 @@ alock_write_slot ( alock_info_t * info, static int alock_query_slot ( alock_info_t * info ) { - int res; + int res, nosave; alock_slot_t slot_data; assert (info != NULL); @@ -302,19 +302,22 @@ alock_query_slot ( alock_info_t * info ) if (slot_data.al_appname != NULL) free (slot_data.al_appname); slot_data.al_appname = NULL; - if (slot_data.al_lock == ALOCK_UNLOCKED) return ALOCK_UNLOCKED; + nosave = slot_data.al_lock & ALOCK_NOSAVE; + + if ((slot_data.al_lock & ALOCK_SMASK) == ALOCK_UNLOCKED) + return slot_data.al_lock; res = alock_test_lock (info->al_fd, info->al_slot); if (res < 0) return -1; if (res > 0) { - if (slot_data.al_lock == ALOCK_UNIQUE) { - return ALOCK_UNIQUE; + if ((slot_data.al_lock & ALOCK_SMASK) == ALOCK_UNIQUE) { + return slot_data.al_lock; } else { - return ALOCK_LOCKED; + return ALOCK_LOCKED | nosave; } } - return ALOCK_DIRTY; + return ALOCK_DIRTY | nosave; } int @@ -328,12 +331,12 @@ alock_open ( alock_info_t * info, alock_slot_t slot_data; char * filename; int res, max_slot; - int dirty_count, live_count; + int dirty_count, live_count, nosave; assert (info != NULL); assert (appname != NULL); assert (envdir != NULL); - assert (locktype >= 1 && locktype <= 2); + assert ((locktype & ALOCK_SMASK) >= 1 && (locktype & ALOCK_SMASK) <= 2); slot_data.al_lock = locktype; slot_data.al_stamp = time(NULL); @@ -370,6 +373,7 @@ alock_open ( alock_info_t * info, max_slot = (statbuf.st_size + ALOCK_SLOT_SIZE - 1) / ALOCK_SLOT_SIZE; dirty_count = 0; live_count = 0; + nosave = 0; scan_info.al_fd = info->al_fd; for (scan_info.al_slot = 1; scan_info.al_slot < max_slot; @@ -377,6 +381,10 @@ alock_open ( alock_info_t * info, if (scan_info.al_slot != info->al_slot) { res = alock_query_slot (&scan_info); + if (res & ALOCK_NOSAVE) { + nosave = ALOCK_NOSAVE; + res ^= ALOCK_NOSAVE; + } if (res == ALOCK_UNLOCKED && info->al_slot == 0) { info->al_slot = scan_info.al_slot; @@ -429,8 +437,8 @@ alock_open ( alock_info_t * info, return ALOCK_UNSTABLE; } - if (dirty_count) return ALOCK_RECOVER; - return ALOCK_CLEAN; + if (dirty_count) return ALOCK_RECOVER | nosave; + return ALOCK_CLEAN | nosave; } int @@ -439,7 +447,7 @@ alock_scan ( alock_info_t * info ) struct stat statbuf; alock_info_t scan_info; int res, max_slot; - int dirty_count, live_count; + int dirty_count, live_count, nosave; assert (info != NULL); @@ -460,12 +468,18 @@ alock_scan ( alock_info_t * info ) max_slot = (statbuf.st_size + ALOCK_SLOT_SIZE - 1) / ALOCK_SLOT_SIZE; dirty_count = 0; live_count = 0; + nosave = 0; for (scan_info.al_slot = 1; scan_info.al_slot < max_slot; ++ scan_info.al_slot) { if (scan_info.al_slot != info->al_slot) { res = alock_query_slot (&scan_info); + if (res & ALOCK_NOSAVE) { + nosave = ALOCK_NOSAVE; + res ^= ALOCK_NOSAVE; + } + if (res == ALOCK_LOCKED) { ++live_count; @@ -491,11 +505,11 @@ alock_scan ( alock_info_t * info ) close (info->al_fd); return ALOCK_UNSTABLE; } else { - return ALOCK_RECOVER; + return ALOCK_RECOVER | nosave; } } - return ALOCK_CLEAN; + return ALOCK_CLEAN | nosave; } int @@ -523,7 +537,7 @@ alock_close ( alock_info_t * info ) free (slot_data.al_appname); return ALOCK_UNSTABLE; } - slot_data.al_lock = ALOCK_UNLOCKED; + slot_data.al_lock = ALOCK_UNLOCKED | (slot_data.al_lock & ALOCK_NOSAVE); res = alock_write_slot (info, &slot_data); if (res == -1) { close (info->al_fd); @@ -584,7 +598,7 @@ alock_recover ( alock_info_t * info ) scan_info.al_slot < max_slot; ++ scan_info.al_slot) { if (scan_info.al_slot != info->al_slot) { - res = alock_query_slot (&scan_info); + res = alock_query_slot (&scan_info) & ~ALOCK_NOSAVE; if (res == ALOCK_LOCKED || res == ALOCK_UNIQUE) { diff --git a/servers/slapd/alock.h b/servers/slapd/alock.h index 49efcbeea0..9225358d48 100644 --- a/servers/slapd/alock.h +++ b/servers/slapd/alock.h @@ -38,6 +38,11 @@ #define ALOCK_UNIQUE (2) #define ALOCK_DIRTY (3) +#define ALOCK_SMASK 3 + +/* lock/state where recovery is not available */ +#define ALOCK_NOSAVE 4 + /* constants */ #define ALOCK_SLOT_SIZE (1024) #define ALOCK_SLOT_IATTRS (4) diff --git a/servers/slapd/back-bdb/init.c b/servers/slapd/back-bdb/init.c index 13415f1e1f..4c41aa1d56 100644 --- a/servers/slapd/back-bdb/init.c +++ b/servers/slapd/back-bdb/init.c @@ -59,6 +59,7 @@ bdb_db_init( BackendDB *be ) bdb->bi_dbenv_mode = SLAPD_DEFAULT_DB_MODE; bdb->bi_cache.c_maxsize = DEFAULT_CACHE_SIZE; + bdb->bi_cache.c_minfree = 1; bdb->bi_lock_detect = DB_LOCK_DEFAULT; bdb->bi_search_stack_depth = DEFAULT_SEARCH_STACK_DEPTH; @@ -94,7 +95,8 @@ bdb_db_open( BackendDB *be ) u_int32_t flags; char path[MAXPATHLEN]; char *dbhome; - int do_recover = 0, do_alock_recover = 0, open_env = 1, got_env = 0; + int do_recover = 0, do_alock_recover = 0, open_env = 1; + int alockt, quick = 0; if ( be->be_suffix == NULL ) { Debug( LDAP_DEBUG_ANY, @@ -125,11 +127,19 @@ bdb_db_open( BackendDB *be ) } /* Perform database use arbitration/recovery logic */ + alockt = (slapMode & SLAP_TOOL_READONLY) ? ALOCK_LOCKED : ALOCK_UNIQUE; + if ( slapMode & SLAP_TOOL_QUICK ) { + alockt |= ALOCK_NOSAVE; + quick = 1; + } + rc = alock_open( &bdb->bi_alock_info, "slapd", - bdb->bi_dbenv_home, - slapMode & SLAP_TOOL_READONLY ? - ALOCK_LOCKED : ALOCK_UNIQUE ); + bdb->bi_dbenv_home, alockt ); + + /* alockt is TRUE if the existing environment was created in Quick mode */ + alockt = (rc & ALOCK_NOSAVE) ? 1 : 0; + rc &= ~ALOCK_NOSAVE; if( rc == ALOCK_RECOVER ) { Debug( LDAP_DEBUG_ANY, @@ -137,7 +147,7 @@ bdb_db_open( BackendDB *be ) " attempting recovery.\n", 0, 0, 0 ); do_alock_recover = 1; - do_recover = 1; + do_recover = DB_RECOVER; } else if( rc == ALOCK_BUSY ) { Debug( LDAP_DEBUG_ANY, "bdb_db_open: database already in use\n", @@ -166,7 +176,7 @@ bdb_db_open( BackendDB *be ) "bdb_db_open: DB_CONFIG for suffix %s has changed.\n" "Performing database recovery to activate new settings.\n", be->be_suffix[0].bv_val, 0, 0 ); - do_recover = 1; + do_recover = DB_RECOVER; } } } @@ -179,6 +189,28 @@ bdb_db_open( BackendDB *be ) bdb->bi_dbenv_home, errno, be->be_suffix[0].bv_val ); } + /* Always let slapcat run, regardless of environment state. + * This can be used to cause a cache flush after an unclean + * shutdown. + */ + if ( do_recover && ( slapMode & SLAP_TOOL_READONLY )) { + Debug( LDAP_DEBUG_ANY, + "bdb_db_open: Recovery skipped in read-only mode. " + "Run manual recovery if errors are encountered.\n", + 0, 0, 0 ); + do_recover = 0; + quick = alockt; + } + + /* An existing environment in Quick mode has nothing to recover. */ + if ( alockt && do_recover ) { + Debug( LDAP_DEBUG_ANY, + "bdb_db_open: cannot recover, database must be reinitialized.\n", + 0, 0, 0 ); + rc = -1; + goto fail; + } + rc = db_env_create( &bdb->bi_dbenv, 0 ); if( rc != 0 ) { Debug( LDAP_DEBUG_ANY, @@ -187,6 +219,27 @@ bdb_db_open( BackendDB *be ) goto fail; } + /* If existing environment is clean but doesn't support + * currently requested modes, remove it. + */ + if ( !do_recover && ( alockt ^ quick )) { + rc = bdb->bi_dbenv->remove( bdb->bi_dbenv, dbhome, DB_FORCE ); + if ( rc ) { + Debug( LDAP_DEBUG_ANY, + "bdb_db_open: dbenv remove failed: %s (%d)\n", + db_strerror(rc), rc, 0 ); + bdb->bi_dbenv = NULL; + goto fail; + } + rc = db_env_create( &bdb->bi_dbenv, 0 ); + if( rc != 0 ) { + Debug( LDAP_DEBUG_ANY, + "bdb_db_open: db_env_create failed: %s (%d)\n", + db_strerror(rc), rc, 0 ); + goto fail; + } + } + bdb->bi_dbenv->set_errpfx( bdb->bi_dbenv, be->be_suffix[0].bv_val ); bdb->bi_dbenv->set_errcall( bdb->bi_dbenv, bdb_errcall ); @@ -220,177 +273,25 @@ bdb_db_open( BackendDB *be ) "bdb_db_open: dbenv_open(%s)\n", bdb->bi_dbenv_home, 0, 0); - /* Check if there is a usable existing environment */ - flags = DB_JOINENV | DB_THREAD; - - rc = bdb->bi_dbenv->open( bdb->bi_dbenv, dbhome, - flags, bdb->bi_dbenv_mode ); - if( rc == 0 ) { - int flags_ok = 0; - - got_env = 1; - - rc = bdb->bi_dbenv->get_open_flags( bdb->bi_dbenv, &flags ); - if ( rc == 0 ) { - int flag2 = flags & BDB_TXN_FLAGS; - - /* In quick mode, none of these flags are allowed */ - if ( slapMode & SLAP_TOOL_QUICK ) { - if ( !flag2 ) - flags_ok = 1; - } else { - /* In normal mode, all of these flags are required */ - if ( flag2 == BDB_TXN_FLAGS ) - flags_ok = 1; - } - } - - /* In Quick mode, we cannot Recover... */ - if ( slapMode & SLAP_TOOL_QUICK ) { - /* If we need to recover but we had no TXNs, just fail */ - if ( do_recover && flags_ok ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: Database cannot be recovered. " - "Restore from backup!\n", 0, 0, 0); - rc = -1; - goto fail; - } - /* We need to recover, and we had TXN support before: - * Close this env, open a new one with recovery flags. - */ - if ( do_recover ) { - bdb->bi_dbenv->close( bdb->bi_dbenv, 0 ); - bdb->bi_dbenv = NULL; - rc = db_env_create( &bdb->bi_dbenv, 0 ); - if( rc != 0 ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: db_env_create failed: %s (%d)\n", - db_strerror(rc), rc, 0 ); - goto fail; - } - bdb->bi_dbenv->set_errpfx( bdb->bi_dbenv, - be->be_suffix[0].bv_val ); - bdb->bi_dbenv->set_errcall( bdb->bi_dbenv, bdb_errcall ); - rc = bdb->bi_dbenv->open( bdb->bi_dbenv, dbhome, - flags | DB_RECOVER, bdb->bi_dbenv_mode ); - if( rc != 0 ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: recovery failed: %s (%d)\n", - db_strerror(rc), rc, 0 ); - goto fail; - } - do_recover = 0; - } - /* Prev environment had TXN support, get rid of it */ - if ( !flags_ok ) { - bdb->bi_dbenv->close( bdb->bi_dbenv, 0 ); - bdb->bi_dbenv = NULL; - rc = db_env_create( &bdb->bi_dbenv, 0 ); - if( rc != 0 ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: db_env_create failed: %s (%d)\n", - db_strerror(rc), rc, 0 ); - goto fail; - } - bdb->bi_dbenv->remove( bdb->bi_dbenv, dbhome, 0 ); - bdb->bi_dbenv = NULL; - } - /* Normal TXN mode */ - } else { - /* If we need to recover but we had no TXNs, just fail */ - if ( do_recover && !flags_ok ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: Database cannot be recovered. " - "Restore from backup!\n", 0, 0, 0); - rc = -1; - goto fail; - } - /* Prev environment had no TXN support, close it */ - if ( !flags_ok ) { - bdb->bi_dbenv->close( bdb->bi_dbenv, 0 ); - bdb->bi_dbenv = NULL; - do_recover = 1; - } - } - - if ( flags_ok && !do_recover ) { - /* This environment is fine, don't reopen it */ - open_env = 0; - } else { - /* Create a new env that can take the desired settings */ - if ( bdb->bi_dbenv != NULL ) { - bdb->bi_dbenv->close( bdb->bi_dbenv, 0 ); - bdb->bi_dbenv = NULL; - } - rc = db_env_create( &bdb->bi_dbenv, 0 ); - if( rc != 0 ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: db_env_create failed: %s (%d)\n", - db_strerror(rc), rc, 0 ); - goto fail; - } + flags = DB_INIT_MPOOL | DB_CREATE | DB_THREAD; - bdb->bi_dbenv->set_errpfx( bdb->bi_dbenv, be->be_suffix[0].bv_val ); - bdb->bi_dbenv->set_errcall( bdb->bi_dbenv, bdb_errcall ); - bdb->bi_dbenv->set_lk_detect( bdb->bi_dbenv, bdb->bi_lock_detect ); - - /* One long-lived TXN per thread, two TXNs per write op */ - bdb->bi_dbenv->set_tx_max( bdb->bi_dbenv, connection_pool_max * 3 ); - - if( bdb->bi_dbenv_xflags != 0 ) { - rc = bdb->bi_dbenv->set_flags( bdb->bi_dbenv, - bdb->bi_dbenv_xflags, 1); - if( rc != 0 ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: dbenv_set_flags failed: %s (%d)\n", - db_strerror(rc), rc, 0 ); - goto fail; - } - } - } - } + if ( !quick ) + flags |= BDB_TXN_FLAGS; - /* If we need to recover but there was no existing environment, - * then we assume that someone has already manually recovered using - * db_recover. Just ignore it. - */ - if ( do_recover && !got_env ) { - do_recover = 0; - Debug( LDAP_DEBUG_TRACE, - "bdb_db_open: Recovery needed but environment is missing - " - "assuming recovery was done manually...\n", 0, 0, 0 ); + /* If a key was set, use shared memory for the BDB environment */ + if ( bdb->bi_shm_key ) { + bdb->bi_dbenv->set_shm_key( bdb->bi_dbenv, bdb->bi_shm_key ); + flags |= DB_SYSTEM_MEM; } + rc = bdb->bi_dbenv->open( bdb->bi_dbenv, dbhome, + flags | do_recover, bdb->bi_dbenv_mode ); - if ( open_env ) { - flags = DB_INIT_MPOOL | DB_THREAD | DB_CREATE; - if ( !( slapMode & SLAP_TOOL_QUICK )) - flags |= BDB_TXN_FLAGS; - - if ( do_recover ) { - if ( slapMode & SLAP_TOOL_READONLY ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: Recovery skipped in read-only mode. " - "Run manual recovery if errors are encountered.\n", - 0, 0, 0 ); - } else { - flags |= DB_RECOVER; - } - } - - /* If a key was set, use shared memory for the BDB environment */ - if ( bdb->bi_shm_key ) { - bdb->bi_dbenv->set_shm_key( bdb->bi_dbenv, bdb->bi_shm_key ); - flags |= DB_SYSTEM_MEM; - } - - rc = bdb->bi_dbenv->open( bdb->bi_dbenv, dbhome, - flags, bdb->bi_dbenv_mode ); - if( rc != 0 ) { - Debug( LDAP_DEBUG_ANY, - "bdb_db_open: dbenv_open failed: %s (%d)\n", - db_strerror(rc), rc, 0 ); - goto fail; - } + if ( rc ) { + Debug( LDAP_DEBUG_ANY, + "bdb_db_open: Database cannot be %s, err %d. " + "Restore from backup!\n", + do_recover ? "recovered" : "opened", rc, 0); + goto fail; } if ( do_alock_recover && alock_recover (&bdb->bi_alock_info) != 0 ) { @@ -419,7 +320,7 @@ bdb_db_open( BackendDB *be ) flags = DB_THREAD | bdb->bi_db_opflags; #ifdef DB_AUTO_COMMIT - if ( !( slapMode & SLAP_TOOL_QUICK )) + if ( !quick ) flags |= DB_AUTO_COMMIT; #endif @@ -519,7 +420,7 @@ bdb_db_open( BackendDB *be ) goto fail; } - if ( !( slapMode & SLAP_TOOL_QUICK )) { + if ( !quick ) { XLOCK_ID(bdb->bi_dbenv, &bdb->bi_cache.c_locker); } diff --git a/servers/slapd/overlays/ppolicy.c b/servers/slapd/overlays/ppolicy.c index 3403e80847..bed5b4b366 100644 --- a/servers/slapd/overlays/ppolicy.c +++ b/servers/slapd/overlays/ppolicy.c @@ -731,8 +731,10 @@ ppolicy_bind_resp( Operation *op, SlapReply *rs ) m->sml_type = ad_pwdFailureTime->ad_cname; m->sml_desc = ad_pwdFailureTime; m->sml_values = ch_calloc( sizeof(struct berval), 2 ); + m->sml_nvalues = ch_calloc( sizeof(struct berval), 2 ); ber_dupbv( &m->sml_values[0], ×tamp ); + ber_dupbv( &m->sml_nvalues[0], ×tamp ); m->sml_next = mod; mod = m; @@ -778,7 +780,9 @@ ppolicy_bind_resp( Operation *op, SlapReply *rs ) m->sml_type = ad_pwdAccountLockedTime->ad_cname; m->sml_desc = ad_pwdAccountLockedTime; m->sml_values = ch_calloc( sizeof(struct berval), 2 ); + m->sml_nvalues = ch_calloc( sizeof(struct berval), 2 ); ber_dupbv( &m->sml_values[0], ×tamp ); + ber_dupbv( &m->sml_nvalues[0], ×tamp ); m->sml_next = mod; mod = m; } @@ -880,7 +884,9 @@ grace: m->sml_type = ad_pwdGraceUseTime->ad_cname; m->sml_desc = ad_pwdGraceUseTime; m->sml_values = ch_calloc( sizeof(struct berval), 2 ); + m->sml_nvalues = ch_calloc( sizeof(struct berval), 2 ); ber_dupbv( &m->sml_values[0], ×tamp ); + ber_dupbv( &m->sml_nvalues[0], ×tamp ); m->sml_next = mod; mod = m; @@ -1155,7 +1161,7 @@ ppolicy_add( timestamp.bv_len = sizeof(timebuf); slap_timestamp( &now, ×tamp ); - attr_merge_one( op->ora_e, ad_pwdChangedTime, ×tamp, NULL ); + attr_merge_one( op->ora_e, ad_pwdChangedTime, ×tamp, ×tamp ); } } return SLAP_CB_CONTINUE; @@ -1568,7 +1574,9 @@ do_modify: if (pwmop != LDAP_MOD_DELETE) { mods->sml_op = LDAP_MOD_REPLACE; mods->sml_values = (BerVarray) ch_malloc( 2 * sizeof( struct berval ) ); + mods->sml_nvalues = (BerVarray) ch_malloc( 2 * sizeof( struct berval ) ); ber_dupbv( &mods->sml_values[0], ×tamp ); + ber_dupbv( &mods->sml_nvalues[0], ×tamp ); mods->sml_values[1].bv_len = 0; mods->sml_values[1].bv_val = NULL; assert( mods->sml_values[0].bv_val != NULL ); diff --git a/servers/slapd/schemaparse.c b/servers/slapd/schemaparse.c index 6b22bf38eb..f802f1ad59 100644 --- a/servers/slapd/schemaparse.c +++ b/servers/slapd/schemaparse.c @@ -124,17 +124,17 @@ cr_usage( void ) int parse_cr( - const char *fname, - int lineno, - char *line, - char **argv, - ContentRule **scr ) + const char *fname, + int lineno, + char *line, + char **argv, + ContentRule **scr ) { LDAPContentRule *cr; int code; const char *err; - cr = ldap_str2contentrule(line, &code, &err, LDAP_SCHEMA_ALLOW_ALL ); + cr = ldap_str2contentrule( line, &code, &err, LDAP_SCHEMA_ALLOW_ALL ); if ( !cr ) { fprintf( stderr, "%s: line %d: %s before %s\n", fname, lineno, ldap_scherr2str(code), err ); @@ -147,27 +147,36 @@ parse_cr( "%s: line %d: Content rule has no OID\n", fname, lineno ); cr_usage(); - return 1; + code = 1; + goto done; } - code = cr_add(cr,1,scr,&err); + code = cr_add( cr, 1, scr, &err ); if ( code ) { fprintf( stderr, "%s: line %d: %s: \"%s\"\n", - fname, lineno, scherr2str(code), err); - return 1; + fname, lineno, scherr2str( code ), err ); + code = 1; + goto done; } - ldap_memfree(cr); - return 0; +done:; + if ( code ) { + ldap_contentrule_free( cr ); + + } else { + ldap_memfree( cr ); + } + + return code; } int parse_oc( - const char *fname, - int lineno, - char *line, - char **argv, - ObjectClass **soc ) + const char *fname, + int lineno, + char *line, + char **argv, + ObjectClass **soc ) { LDAPObjectClass *oc; int code; @@ -176,7 +185,7 @@ parse_oc( oc = ldap_str2objectclass(line, &code, &err, LDAP_SCHEMA_ALLOW_ALL ); if ( !oc ) { fprintf( stderr, "%s: line %d: %s before %s\n", - fname, lineno, ldap_scherr2str(code), err ); + fname, lineno, ldap_scherr2str( code ), err ); oc_usage(); return 1; } @@ -186,18 +195,27 @@ parse_oc( "%s: line %d: objectclass has no OID\n", fname, lineno ); oc_usage(); - return 1; + code = 1; + goto done; } - code = oc_add(oc,1,soc,&err); + code = oc_add( oc, 1, soc, &err ); if ( code ) { fprintf( stderr, "%s: line %d: %s: \"%s\"\n", - fname, lineno, scherr2str(code), err); - return 1; + fname, lineno, scherr2str( code ), err ); + code = 1; + goto done; } - ldap_memfree(oc); - return 0; +done:; + if ( code ) { + ldap_objectclass_free( oc ); + + } else { + ldap_memfree( oc ); + } + + return code; } static void @@ -245,11 +263,11 @@ at_usage( void ) int parse_at( - const char *fname, - int lineno, - char *line, - char **argv, - AttributeType **sat ) + const char *fname, + int lineno, + char *line, + char **argv, + AttributeType **sat ) { LDAPAttributeType *at; int code; @@ -268,23 +286,33 @@ parse_at( "%s: line %d: attributeType has no OID\n", fname, lineno ); at_usage(); - return 1; + code = 1; + goto done; } /* operational attributes should be defined internally */ if ( at->at_usage ) { fprintf( stderr, "%s: line %d: attribute type \"%s\" is operational\n", fname, lineno, at->at_oid ); - return 1; + code = 1; + goto done; } - code = at_add(at,1,sat,&err); + code = at_add( at, 1, sat, &err); if ( code ) { - ldap_attributetype_free( at ); fprintf( stderr, "%s: line %d: %s: \"%s\"\n", fname, lineno, scherr2str(code), err); - return 1; + code = 1; + goto done; } - ldap_memfree(at); - return 0; + +done:; + if ( code ) { + ldap_attributetype_free( at ); + + } else { + ldap_memfree( at ); + } + + return code; } -- 2.39.5