Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> On Fri, Jul 2, 2010 at 07:54, László ÁSHIN <laszlo.ashin@xxxxxxx> wrote:
>
> > +
> > + open SCRIPTIN, '|' . $authscript or die $!;
> > + print SCRIPTIN $user . "\n";
> > + print SCRIPTIN descramble($password) . "\n";
> > + close SCRIPTIN;
>
> Nit: Nice use of three-arg open, but you should use lexical
> filehandles instead. I.e.:
>
> open my $script, '|' . $authscript or die $!;
> ...
This is two-argument open, not three-argument magic open. There is
string concatenation operator '.' there, not a comma ',' delimiting
arguments.
It should be
open my $script_fd, '|-', $authscript
or die "Couldn't open authentication script '$authscript': $!";
> > + } else {
> > + if (not exists $cfg->{gitcvs}->{authdb}) {
Why not elsif?
> > + print "E the repo config file needs a [gitcvs] section with an 'authdb' parameter set to the filename of the authentication database\n";
Overly long line. Perhaps it would be better to split it into
concatenated parts.
> > + my $auth_ok;
> > + open my $passwd, "<", $authdb or die $!;
And here you use three-argument form of (ordinary) open.
> > + while (<$passwd>) {
> > + if (m{^\Q$user\E:(.*)}) {
> > + if (crypt($user, descramble($password)) eq $1) {
Why nested if, and not short-circuit &&?
+ if (/^\Q$user\E:(.*)/ &&
+ crypt($user, descramble($password)) eq $1) {
--
Jakub Narebski
Poland
ShadeHawk on #git
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html