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

[Torsten Landschoff]

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 :-(

> +			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.

> +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... :)

 
Greetings

	Torsten
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.alioth.debian.org/pipermail/pkg-openldap-devel/attachments/20060603/7779f3ba/attachment.pgp