threads / patch / 24305

patch, 5 partsgit-cvsserver: break the loop after successful hit

Subject: [PATCH 4/5] git-cvsserver: break the loop after successful hit

## tl;dr

2 messages between Jul 6, 2010 and Jul 7, 2010. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Áshin László· Jul 6, 2010, 17:38 UTC · lore

No need to check for more lines of the authentication database after found a
matching one.
---
 git-cvsserver.perl |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

Show changes to git-cvsserver.perl +4 −6

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index e842d77..cd461ac 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -248,11 +248,10 @@ if ($state->{method} eq 'pserver') {
        my $auth_ok;
        open my $passwd, "<", $authdb or die $!;
        while (<$passwd>) {
-            if (m{^\Q$user\E:(.*)}) {
-                if (crypt($user, $password) eq $1) {
-                    $auth_ok = 1;
-                }
-            };
+            if (m{^\Q$user\E:(.*)} && crypt($user, $password) eq $1) {
+                $auth_ok = 1;
+                last;
+            }
        }
        close $passwd;

--
1.7.0.4

Junio C Hamano· Jul 7, 2010, 05:28 UTC · re: Áshin László · lore

Re: [PATCH 4/5] git-cvsserver: break the loop after successful hit

Áshin László <ashinlaszlo@gmail.com> writes:

> No need to check for more lines of the authentication database after found a
> matching one.

s/found/finding/;

But I see a bigger issue here. Is it plausible that you might want to allow two entries for one user, with two different passwords, in an authdb file? The current code allows it, but if we want to disallow it, we could instead add your "last", like this, if you do not break the structure of the conditional statements in the original code.

	while (<$passwd>) {
		if (m{^\Q$user\E:(.*)}) {
			if (crypt($user, $password) eq $1) {
				$auth_ok = 1;
			}
			last;
		}
	}

Or have the "last" inside the inner "if" statement to keep that logic.

Either way I think the original nested "if" is much easier to read than your version.