Closed
Bug 351020
Opened 18 years ago
Closed 18 years ago
ldap_add_result_entry library entry is LOCL in libldap.so
Categories
(Directory :: LDAP C SDK, enhancement)
Directory
LDAP C SDK
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jiri.sasek, Assigned: neuroc0der)
References
Details
Attachments
(2 files, 1 obsolete file)
4.12 KB,
patch
|
Details | Diff | Splinter Review | |
1.17 KB,
patch
|
Details | Diff | Splinter Review |
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•18 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•18 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•18 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•18 years ago
|
||
Comment 5•18 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•18 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•18 years ago
|
||
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•18 years ago
|
||
I think the new diff looks good. Do we really care about what the Win32 function ordinal numbers are?
Comment 9•18 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•18 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•18 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•18 years ago
|
||
i had Doze build failed last night and this is to make VC6 happy.
Assignee | ||
Comment 13•18 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
Comment 14•18 years ago
|
||
This has been merged on to the trunk. We can resolve this as FIXED.
Comment 15•18 years ago
|
||
Reassigned to Anton (for tracking purposes) and marked fixed.
Assignee: mcs → anton.bobrov
Updated•18 years ago
|
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•