Issue 8011 - few mistakes in lmdb-backend
Summary: few mistakes in lmdb-backend
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-28 20:52 UTC by Leonid Yuriev
Modified: 2015-07-02 17:46 UTC (History)
0 users

See Also:


Attachments
its8011.patch (2.16 KB, patch)
2014-12-29 08:02 UTC, Leonid Yuriev
Details
ITS8011.patch (2.75 KB, patch)
2014-12-28 20:56 UTC, Leonid Yuriev
Details

Note You need to log in before you can comment on or make changes to this issue.
Description Leonid Yuriev 2014-12-28 20:52:21 UTC
Full_Name: Leonid Yuriev
Version: master
OS: RHEL7
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (31.130.36.33)


Please review the patch, which will be attached shortly.

mdb_idscopes():
  + assertion for mdb_id2l_insert() result;
  - unnecessary assignment;
    
static search_aliases():
  - unused variable 'first';
  + range-check for mdb_idl_search() result;

static search_stack():
  * fix sctmp/stack allocation size;
Comment 1 Leonid Yuriev 2014-12-28 20:56:55 UTC
The attached files is derived from OpenLDAP Software. All of the modifications
to OpenLDAP Software represented in the following patch(es) were developed by
Peter-Service LLC, Moscow, Russia. Peter-Service LLC has not assigned rights
and/or interest in this work to any party. I, Leonid Yuriev am authorized by
Peter-Service LLC, my employer, to release this work under the following terms.

Peter-Service LLC hereby places the following modifications to OpenLDAP Software
(and only these modifications) into the public domain. Hence, these
modifications may be freely used and/or redistributed for any purpose
with or without attribution and/or other notice.


Comment 2 Howard Chu 2014-12-28 22:13:38 UTC
leo@yuriev.ru wrote:
> @@ -1265,8 +1264,10 @@ static void *search_stack( Operation *op )
>   	}
>
>   	if ( !ret ) {
> -		ret = ch_malloc( mdb->mi_search_stack_depth * MDB_IDL_UM_SIZE
> -			* sizeof( ID ) );
> +		size_t case_stack = mdb->mi_search_stack_depth * MDB_IDL_UM_SIZE * sizeof( ID );
> +		size_t case_sctmp = MDB_IDL_UM_SIZE * sizeof( ID2 );
> +		size_t size = (case_stack > case_sctmp) ? case_stack : case_sctmp;
> +		ret = ch_malloc( size );
>   		if ( op->o_threadctx ) {
>   			ldap_pvt_thread_pool_setkey( op->o_threadctx, (void *)search_stack,
>   				ret, search_stack_free, NULL, NULL );
>

Unnecessary.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 3 Leonid Yuriev 2014-12-29 08:02:51 UTC
Updated.
Please pickup.

Leonid.

2014-12-29 1:13 GMT+03:00 Howard Chu <hyc@symas.com>:

> leo@yuriev.ru wrote:
>
>> @@ -1265,8 +1264,10 @@ static void *search_stack( Operation *op )
>>         }
>>
>>         if ( !ret ) {
>> -               ret = ch_malloc( mdb->mi_search_stack_depth *
>> MDB_IDL_UM_SIZE
>> -                       * sizeof( ID ) );
>> +               size_t case_stack = mdb->mi_search_stack_depth *
>> MDB_IDL_UM_SIZE * sizeof( ID );
>> +               size_t case_sctmp = MDB_IDL_UM_SIZE * sizeof( ID2 );
>> +               size_t size = (case_stack > case_sctmp) ? case_stack :
>> case_sctmp;
>> +               ret = ch_malloc( size );
>>                 if ( op->o_threadctx ) {
>>                         ldap_pvt_thread_pool_setkey( op->o_threadctx,
>> (void *)search_stack,
>>                                 ret, search_stack_free, NULL, NULL );
>>
>>
> Unnecessary.
>
> --
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP  http://www.openldap.org/project/
>
Comment 4 Howard Chu 2015-01-04 07:39:49 UTC
Леонид Юрьев wrote:
> Updated.
> Please pickup.

> diff --git a/servers/slapd/back-mdb/dn2id.c b/servers/slapd/back-mdb/dn2id.c
> index 41c4758..0890bce 100644
> --- a/servers/slapd/back-mdb/dn2id.c
> +++ b/servers/slapd/back-mdb/dn2id.c
> @@ -746,7 +746,8 @@ mdb_idscopes(
>  			/* remember our chain of parents */
>  			id2.mid = id;
>  			id2.mval = data;
> -			mdb_id2l_insert( isc->sctmp, &id2 );
> +			rc = mdb_id2l_insert( isc->sctmp, &id2 );
> +			assert(rc == 0);
>  		}
>  		ptr = data.mv_data;
>  		ptr += data.mv_size - sizeof(ID);

Unnecessary, the total number of parents is constrained by the max 
length of a DN which is 8192 bytes. A maximum depth DN (max number of 
shortest possible RDNs) will always fit in sctmp.

I will commit the remainder of the patch.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 5 Howard Chu 2015-01-04 07:42:44 UTC
changed notes
changed state Open to Test
moved from Incoming to Software Bugs
Comment 6 Quanah Gibson-Mount 2015-01-05 20:00:14 UTC
changed notes
changed state Test to Release
Comment 7 OpenLDAP project 2015-07-02 17:46:06 UTC
fixed in master
fixed in RE25
fixed in RE24
Comment 8 Quanah Gibson-Mount 2015-07-02 17:46:06 UTC
changed notes
changed state Release to Closed