364812 - merging Sun and Mozilla ldap tools.

Status: RESOLVED FIXED

(Directory :: LDAP C SDK, defect)

Description

[Anton Bobrov]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1

arent that many changes so hopefully should be fast to review and 
integrate, plz see the next update for the first cut of the patch
.


Reproducible: Always

Comment 1

[Anton Bobrov]

Attachment: the first cut of the patch

- removed FORTEZZA
- removed LIBCACHE and [-C] no longer referenced in common
- added getEffectiveRights control to ldapsearch
- deprecated [-X] now utilized for getEffectiveRights control
- [-c] utilized for getEffectiveRights control 
- added [-W -] for SSL password prompt
- added user friendly help options
- added [-i 0] to completely bypass charset conversions 
- various minor fixes and improvements

this patch has been successfully tested on Solaris, 
would appreciate review asap. merry xmas and happy 
new year! :)

Comment 2

[Rich Megginson]

The indentation looks a bit dodgy here - https://bugzilla.mozilla.org/attachment.cgi?id=249516&action=diff#mozilla/directory/c-sdk/ldap/clients/tools/ldapsearch.c_sec8

How is the get effective rights attr list passed into the tool?  What is the delimiter character?

Comment 3

[Anton Bobrov]

they are space [" "] separated eg -X "mail uid". it might be worth a mention in
the help output but i figured its pretty intuitive tho i guess if you ask it is
certainly not. should i add appropriate line to the help output then ?

(In reply to comment #2)
> The indentation looks a bit dodgy here -
> https://bugzilla.mozilla.org/attachment.cgi?id=249516&action=diff#mozilla/directory/c-sdk/ldap/clients/tools/ldapsearch.c_sec8
> 
> How is the get effective rights attr list passed into the tool?  What is the
> delimiter character?
> 

Comment 4

[Rich Megginson]

(In reply to comment #3)
> they are space [" "] separated eg -X "mail uid". it might be worth a mention in
> the help output but i figured its pretty intuitive tho i guess if you ask it is
> certainly not. should i add appropriate line to the help output then ?

Sure.  Just add something to the help output to indicate the delimiter is a space character.  Otherwise, looks good.

Comment 5

[Anton Bobrov]

Attachment: rev 1 of the original patch

thanx for review Rich! in this patch there is an explicit comment 
re using space as separator in getEffectiveRights attributes list
.

Comment 6

[Rich Megginson]

Ok.  Looks good.

Comment 7

[Mark Smith [:mcs]]

Comment on attachment 249988 [details] [diff] [review]
rev 1 of the original patch

This looks good, except I am not sure about the changes to fileurl.c.  I think the old code was designed to handle file:/path or file://path.  With your proposed change, file:/path will no longer work (yes, that format is somewhat of a shorthand but people expect it to work).

I also would not mind of get_effectiverights_attrlist() checked the return value from functions that need to allocate memory.

Comment 8

[Anton Bobrov]

(In reply to comment #7)

> the old code was designed to handle file:/path or file://path.  With your
> proposed change, file:/path will no longer work (yes, that format is somewhat
> of a shorthand but people expect it to work).

ok this makes sense. should i change back both the if statement and comments 
or just the if statement ?
 
> I also would not mind of get_effectiverights_attrlist() checked the return
> value from functions that need to allocate memory.

sure, will have new rev of patch covering these comments available tomorrow. 

Comment 9

[Anton Bobrov]

pardon me, instead of "comments" in the first part of Comment #8 
please read "error messages". its getting kinda late here, sorry
.

Comment 10

[Mark Smith [:mcs]]

(In reply to comment #9)
> pardon me, instead of "comments" in the first part of Comment #8 
> please read "error messages".

I think your error message changes are OK.  Just revert the if statements.

Comment 11

[Anton Bobrov]

Attachment: rev 2 of the original patch

this should be it, please review.

Comment 12

[Mark Smith [:mcs]]

Comment on attachment 251068 [details] [diff] [review]
rev 2 of the original patch

Thanks for making the changes.  This looks good to me now.  Please commit.

Comment 13

[Anton Bobrov]

done. thanx for reviews!

Checking in mozilla/directory/c-sdk/ldap/clients/tools/Options.txt;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/Options.txt,v  <--  Options.txt
new revision: 5.7; previous revision: 5.6
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/common.c;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/common.c,v  <--  common.c
new revision: 5.22; previous revision: 5.21
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/convutf8.cpp;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/convutf8.cpp,v  <--  convutf8.cpp
new revision: 5.9; previous revision: 5.8
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/fileurl.c;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/fileurl.c,v  <--  fileurl.c
new revision: 5.5; previous revision: 5.4
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/ldapcmp.c;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/ldapcmp.c,v  <--  ldapcmp.c
new revision: 5.7; previous revision: 5.6
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/ldapcompare.c;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/ldapcompare.c,v  <--  ldapcompare.c
new revision: 5.8; previous revision: 5.7
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/ldapmodrdn.c;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/ldapmodrdn.c,v  <--  ldapmodrdn.c
new revision: 5.3; previous revision: 5.2
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/ldapsearch.c;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/ldapsearch.c,v  <--  ldapsearch.c
new revision: 5.12; previous revision: 5.11
done
Checking in mozilla/directory/c-sdk/ldap/clients/tools/ldaptool.h;
/cvsroot/mozilla/directory/c-sdk/ldap/clients/tools/ldaptool.h,v  <--  ldaptool.h
new revision: 5.9; previous revision: 5.8
done