[Date Prev][Date Next] [Chronological] [Thread] [Top]

(ITS#8645) syncrepl configuration error handling



Full_Name: Emmanuel L.charny
Version: 
OS: 
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (176.154.3.51)


The parse_syncrepl_line function (syncrepl.c) returns either 0, -1 or 1. 0 is
for success, -1 is for a error, 1 seems to be for a warning (ie a parameter is
wrongly configured, but then it's ignored).

That's ok, except that in the loop where the parameters are parsed, if one of
the parameters in {'retry', 'manageDSAit', 'sizelimit', 'timelimit'} is
incorect, the following parameters aren't parsed at all, the fuction return 1. 

At this point, in the caller function add_syncrepl, we have :

    rc = parse_syncrepl_line( c, si );

    if ( rc == 0 )
    {
        ...
    }

    if ( rc < 0 )
    {
        Debug( LDAP_DEBUG_ANY, "failed to add syncinfo\n", 0, 0, 0 );
        syncinfo_free( si, 0 );
        
        return 1;
    }
    else
    {
        Debug( LDAP_DEBUG_CONFIG,
               "Config: ** successfully added syncrepl %s \"%s\"\n",
               si->si_ridtxt,
               BER_BVISNULL( &si->si_bindconf.sb_uri ) ?
               "(null)" : si->si_bindconf.sb_uri.bv_val, 0 );
    ...

So here, we will have a message indicating syncrepl has properly be initialized,
when many parameters might have been ignored.

It seems the proper handling of those wrongly initialized parameters should be
to do a 'continue' instead of returning 1, and add a warning in logs. Like :

    ...
    else if ( !strncasecmp( c->argv[ i ], RETRYSTR "=", STRLENOF( RETRYSTR "=" )
) )
    {
        if ( parse_syncrepl_retry( c, c->argv[ i ], si ) )
        {
            continue;   /* was return 1;
        }
    }

Or maybe we just want to return -1 in all cases ?