[Pkg-openldap-devel] r669 - openldap/trunk-2.3/debian

[Matthijs Mohlmann]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Torsten Landschoff wrote:
> Hi Matthijs, 
> 
> On Fri, Jun 02, 2006 at 08:34:15PM +0000, Matthijs Mohlmann wrote:
>  
>> Modified: openldap/trunk-2.3/debian/slapd.scripts-common
>> ===================================================================
>> --- openldap/trunk-2.3/debian/slapd.scripts-common	2006-06-01 20:28:42 UTC (rev 668)
>> +++ openldap/trunk-2.3/debian/slapd.scripts-common	2006-06-02 20:34:14 UTC (rev 669)
>> @@ -131,9 +131,38 @@
>>  }
>>  
>>  # }}}
>> -
>> -
>> +create_new_user() { # {{{
>> +	if [ "$MODE" = "configure" ]; then
> 
> I'd suggest moving that check to the caller, otherwise it is hard to
> follow the control flow of the postinst. I am not sure if there are 
> functions left over from my work that do that - just wanted to check,
> but blame annotation has forgot about my work :-(
> 
Agreed with the suggestion. :)

>> +			adduser --quiet --system --home /var/lib/ldap --shell /bin/false --ingroup openldap --disabled-password --disabled-login --gecos "OpenLDAP" openldap
> 
> I'd suggest splitting this into multiple lines. My window currently has
> some 132x40 lines, but it still does not fit on the screen ;) Also I
> think the gecos field should be more verbose, like "OpenLDAP Server
> Account" or something like that.
> 
Also agreed, and changed the gecos to "OpenLDAP Server Account".

>> +create_ldap_directories() {	# {{{
>> +	if [ ! -d /var/lib/ldap && ! -z $SLAPD_USER && ! -z $SLAPD_GROUP ]; then
> 
> Why use test ! -z instead of test -n? I am also not sure if this would
> work. This should be either
> 
>   if [ ! -d /var/lib/ldap ] && [ -n $SLAPD_USER ] && [ -n $SLAPD_GROUP ];
> 
> which also looks dangerous as it will not work if $SLAPD_USER is empty.
> How about
> 
>   if [ ! -d /var/lib/ldap -a -n "$SLAPD_USER" -a -n "$SLAPD_GROUP" ]; then
> 
> I still can not see why we'd require both SLAPD_USER and SLAPD_GROUP to
> be set for creating the directory. What is this code supposed to do?
> 
>> +update_permissions() {	# {{{
> 
> Same comments... :)
> 
This is still buggy code, that's why I didn't make use of it yet in the
scripts. The create_ldap_directories should make the directories with
the permissions of $SLAPD_USER and $SLAPD_GROUP if not empty.

>  
> Greetings
> 
> 	Torsten

Regards,

Matthijs Mohlmann

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEgWtD2n1ROIkXqbARAnc3AJ97M3o+aQkhEGhtqrgGxgsB9uG88ACfQE/V
6w/0vUkqH1ArtwK449/sPyI=
=k5us
-----END PGP SIGNATURE-----