ldap_add_result_entry library entry is LOCL in libldap.so

RESOLVED FIXED

Status

Directory
LDAP C SDK
--
enhancement
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: jiri.sasek, Assigned: neuroc0der@gmail.com)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.7) Gecko/20060120
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.7) Gecko/20060120

ldap_add_result_entry can not be linked/called from the user application code. Also the:

nm libraries/libldap/SunOS5.11_i86pc_OPT.OBJ/libldap60.so | less
...is showing:
[671]   |     22832|    1177|FUNC |GLOB |0    |11     |ldap_add_ext
[377]   |     24064|     122|FUNC |GLOB |0    |11     |ldap_add_ext_s
[60]    |     93152|      20|FUNC |LOCL |2    |11     |ldap_add_result_entry
[559]   |     24016|      42|FUNC |GLOB |0    |11     |ldap_add_s
[418]   |     41136|      35|FUNC |GLOB |0    |11     |ldap_ber_free
[398]   |     24592|      87|FUNC |GLOB |0    |11     |ldap_bind

...so the ldap_add_result_entry is not exported by the libldap library

Reproducible: Always

Steps to Reproduce:
1. nm libldap.so
2. look on ldap_add_result_entry
Actual Results:  
[60]    |     93152|      20|FUNC |LOCL |2    |11     |ldap_add_result_entry

Expected Results:  
[60]    |     93152|      20|FUNC |GLOB |2    |11     |ldap_add_result_entry

ldap_add_result_entry have to be named in the .spec file

ldap_add_result_entry is neede by Samba when it is built with the Active Directory support

Comment 1

11 years ago
In OpenLDAP these two functions are public:
  ldap_add_result_entry()
  ldap_delete_result_entry()
(with prototypes in ldap.h and presumably with appropriate exports in shared object/DLL definition files).

I have no problem exposing these.  Rich and Anton, what do you think?
Severity: normal → enhancement
OS: Solaris → All
Hardware: Sun → All
(Assignee)

Comment 2

11 years ago
Mark, you are too fast [good thing]! :) below is the comment i been trying 
to submit before getting into mid-air collision. additionally i wanna note 
that this is for our Samba folks so they can have less broblems building 
with our ldap libs.

Original comment:
=================

Mark, just to give you some more info here. OpenLDAP exposed these two calls

LDAP_F( LDAPMessage * ) 
ldap_delete_result_entry LDAP_P((
        LDAPMessage **list,
        LDAPMessage *e ));
 
LDAP_F( void )
ldap_add_result_entry LDAP_P((
        LDAPMessage **list,
        LDAPMessage *e ));

in their ldap.h while ago, while we, probably for historical reasons, kept 
them private in our ldap-int.h.

i dont think there is any risk making them public so if you dont have any
objections i'm gonna prep a patch for review to make them public. i wanna
apply changes to sun_merge branch so that i can backport the same changes
to the private Sun branch inhouse. dunno if there is a need to have them
on the main branch right now tho.

Comment 3

11 years ago
Yes, let's make them public.  We are testing the last batch of sun merge branch code now.  We should pick up a new drop soon - it would be good to get these in.  Once we test them, we'll put them on the trunk.
(Assignee)

Comment 4

11 years ago
Created attachment 236692 [details] [diff] [review]
patch rev 0

Comment 5

11 years ago
Comment on attachment 236692 [details] [diff] [review]
patch rev 0


>+580 ldap_delete_result_entry
>+581 ldap_add_result_entry

What process are we using to assign ordinal numbers?  I assume it is hopeless to match those used by OpenLDAP, assuming they still assign numbers?


>Index: mozilla/directory/c-sdk/ldap/libraries/libldap/reslist.c
> ...
> void
> ldap_add_result_entry( LDAPMessage **list, LDAPMessage *e )
> {
>+    if ( list != NULL || e != NULL ) {
> 	e->lm_chain = *list;
> 	*list = e;
>+    }
> }

The check should be:
  if ( list != NULL && e != NULL ) {
(replace || with &&).
(Assignee)

Comment 6

11 years ago
(In reply to comment #5)

> >+580 ldap_delete_result_entry
> >+581 ldap_add_result_entry
> 
> What process are we using to assign ordinal numbers?  I assume it is hopeless
> to match those used by OpenLDAP, assuming they still assign numbers?

first off i dunno if OpenLDAP is actually using any mapfiles or at least i 
dont see any at the first glance.

regarding the numbers themselves i dont think they actually mean anything
[ or is it something Doze specific ? ] the only reason i put numbers there 
is because i'm not familiar with gen export script and thought it might 
depend on having them as first field. anyways if they are still used eg on
Doze i dont see any precise pattern or rule that could have been used to 
assign those numbers apart from the likes of pick some unused number which
is a bit away from the last used number which is what i have done as well.

> The check should be:
>   if ( list != NULL && e != NULL ) {
> (replace || with &&).

yep, nasty cut & paste on my part, sorry about that and thanx for spot on. 
(Assignee)

Comment 7

11 years ago
Created attachment 237105 [details] [diff] [review]
patch rev 1

fixed "if" statement as suggested.

Mark, Rich, any further updates/opinions/newintel on what you wanna do
regarding mapfile numbers here ?
Attachment #236692 - Attachment is obsolete: true

Comment 8

11 years ago
I think the new diff looks good.  Do we really care about what the Win32 function ordinal numbers are?

Comment 9

11 years ago
(In reply to comment #8)
> I think the new diff looks good.  Do we really care about what the Win32
> function ordinal numbers are?

Probably not.  My only concern is that if they are used by the dynamic loader on Windows (and I am not sure if they still are or not but I think so), then it would be nice to use the same numbers as other LDAP C API implementations... but we have other incompatibilities anyway, so let's plow ahead.
(Assignee)

Comment 10

11 years ago
yeah. the thing is, the only situation i can imagine were having the same
numbers would be important is when one wants to drop mozilla libldap bin
in place of openldap libldap bin, without rebuilding. i dont see anything
similar in openldap tree today and i'm not aware of any other open source
implementation/s were we can look it up to stay on common ground. due to
other various diffs in implementations i reckon its unrealistic to expect
sucha drop replacement approach in the near future and if we ever reach
the point were that would be possible and *if* this doze optimization get
in our way we can revisit it then. i will go ahead and ci on sun_merge.  

(In reply to comment #9)
> Probably not.  My only concern is that if they are used by the dynamic loader
> on Windows (and I am not sure if they still are or not but I think so), then it
> would be nice to use the same numbers as other LDAP C API implementations...
> but we have other incompatibilities anyway, so let's plow ahead.
(Assignee)

Comment 11

11 years ago
Mark, Rich, once again, thanx for review!

Checking in mozilla/directory/c-sdk/ldap/include/ldap-extension.h;
/cvsroot/mozilla/directory/c-sdk/ldap/include/ldap-extension.h,v  <--  ldap-extension.h
new revision: 5.4.8.5; previous revision: 5.4.8.4
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap.ex;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap.ex,v  <--  libldap.ex
new revision: 5.2.8.3; previous revision: 5.2.8.2
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/ldap-int.h;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/ldap-int.h,v  <--  ldap-int.h
new revision: 5.6.2.4; previous revision: 5.6.2.3
done
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/reslist.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/reslist.c,v  <--  reslist.c
new revision: 5.2.8.1; previous revision: 5.2
done
(Assignee)

Comment 12

11 years ago
Created attachment 237949 [details] [diff] [review]
consistent func declaration

i had Doze build failed last night and this is to make VC6 happy.
(Assignee)

Comment 13

11 years ago
Checking in mozilla/directory/c-sdk/ldap/libraries/libldap/reslist.c;
/cvsroot/mozilla/directory/c-sdk/ldap/libraries/libldap/reslist.c,v  <--  reslist.c
new revision: 5.2.8.2; previous revision: 5.2.8.1
done
(Assignee)

Updated

11 years ago
Blocks: 339298

Comment 14

11 years ago
This has been merged on to the trunk.  We can resolve this as FIXED.

Comment 15

11 years ago
Reassigned to Anton (for tracking purposes) and marked fixed.
Assignee: mcs → anton.bobrov

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.