Re: [patch] keepalive=>1

[Graham B. <gb...@po...>]

On Fri, Jun 21, 2002 at 10:34:36AM +0100, Chris Ridd wrote:
> >     116 sub _connect {
> >     117   my ($ldap, $host, $arg) = @_;
> >     118 
> >     119   $ldap->{net_ldap_socket} = IO::Socket::INET->new(
> >     120     PeerAddr => $host,
> >     121     PeerPort => $arg->{port} || '389',
> >     122     Proto    => 'tcp',
> >     123     Timeout  => defined $arg->{timeout}
> >     124                  ? $arg->{timeout}
> >     125                  : 120
> >     126   );
> > ++  127   $ldap->{net_ldap_socket}->sockopt(SO_KEEPALIVE,1)
> > ++  128         if defined $arg->{keepalive};
> >     129 
> >     130 }
> 
> That looks OK to me.

I am not so sure. Other than the defined should not be there as
it means   keepalive => 0   will do the same as keepalive => 1
which is not right IMO.

But Net::LDAP does not hide the socket away, it is avaliable via a method
for this reason. I dont want to cram _connect with all possible options that
people may want. In this case the author could just as easily do

  $ldap = Net::LDAP->new(...);
  $ldap->socket->sockopt(SO_KEEPALIVE,1);

Graham.