http://qs321.pair.com?node_id=862634

weglarz has asked for the wisdom of the Perl Monks concerning the following question:

Hello Perl Monks! I am writing a simple gatekeeper program with a few subroutines. This one I am having trouble with always returns "pass", even though it seems as if the logic is sound. Thanks!

sub entersys { print("Please enter your username: "); chop($euser = <STDIN>); print("Please enter your password: "); chop($epass = <STDIN>); if($userpass{$euser} eq $epass) { $x = 'pass'; } else { $x = 'fail'; } }

Replies are listed 'Best First'.
Re: prob w/ gatekeeper subroutine
by toolic (Bishop) on Sep 29, 2010 at 15:53 UTC
    It always sets $x to 'fail' for me, probably because I do not populate the hash. You should show a complete code example that any of us can run: how you call your sub, how you know if it passes or fails, how you initialize your hash. Also, tell us what you enter on the keyboard once you run it.

      Thanks for the response. I will post the hash and the main part of the code that calls the subroutine. Sorry, should have done this in the first place.

      $x = (); %userpass = ('john','ball','tim','super','matt','password','dan','nad3 +2'); &entersys; if($x == "pass") { print("Welcome to the system!\n"); } else { print("That user/password is incorrect\n"); }

      I use them as global variables and a global hash so that the subroutines permanently change them.

        I use them as global variables and a global hash so that the subroutines permanently change them.

        Don't do that. It's called action at a distance and it's considered a bad practice. Instead of modifying a global variable, you should return a value from entersys().

        (The error regarding the check for numerical vs. string equality has been pointed out and was not contained in your original post, so I ignore it in my response.)

        I believe you are encountering a variable scoping issue. Make sure your hash contents are visible to the subroutine (which they aren't unless you assign the correct global scope to the hash variable). The following declaration of the hash inside a naked block will throw an error as the hash is unknown to the subroutine.
        #! /usr/bin/perl -w use strict; { our %hash; $hash{ 1 } = "OK"; &routine; } sub routine { print $hash{ 1 }; }

        Hope this helps.
        Pat
Re: prob w/ gatekeeper subroutine
by GrandFather (Saint) on Sep 29, 2010 at 20:54 UTC

    As others have suggested: Always use strictures (use strict; use warnings; - see The strictures, according to Seuss)!

    You seem to be writing Perl circa 4.x rather than 5.x. There were many enhancements made to Perl between 4.x and 5.x with most of them improving robustness of code in various ways (using strictures is an example of that). A few things we do differently now:

    • Use chomp instead of chop. chop simply removes the last character - whatever it was. chomp removes the a trailing copy of the contents of $/ (the input record separator special variable), normally a \n.
    • You don't need to initialise variables - Perl always does that by setting them to undef.
    • Don't call subs using &. That has a special meaning and will most often result in surprises - surprises are generally bad.
    • Don't use global variables. In small programs it's not such an issue, but as programs grow global variables make it very difficult to figure out how things work.
    True laziness is hard work

      Thanks for everyone's responses. I am actually using Perl v 5.8.8. Also, what would I call a subroutine with other than &? I am currently learning to use strict but as this is an assignment, I need to get it to work before the due date and I cannot spend the extra time on learning strict before I have to turn it in. Also, if I were to stop using global variables, how would I change the admin password in this program. At the moment I have a subroutine that changes the global variable $adminpass.

        Well, I don't want to write your assignment answer for you, so I'll give you some different code to ponder a while that may help answer some of your questions.

        use strict; use warnings; main (); # Only to ensure no global variables are used - look ma, no v +ariables sub main { my ($dogName, $color) = getDogData (); print "The $color dog's name is $dogName\n"; ($dogName, $color) = getDogData (3); print "The $color dog's name is $dogName\n"; } sub getDogData { my ($select) = @_; my @names = qw(woof k9 fred max); my @colors = qw(spotted white black brown); $select = rand (@names) if ! defined $select; return $names[$select], $colors[$select]; }

        Prints:

        The black dog's name is fred The brown dog's name is max

        Please note that using strictures is not something you do when you have time, it is something you do to save time! Strictures tell you you have done something wrong early and can save huge amounts of time that would otherwise be spent poking around in the dark after black beetles with your eyes closed.

        Update: In a follow up question weglarz asked 'What does the "qw" in "my @names = qw(woof k9 fred max);" do?'. See Quote-Like Operators in perlop for a description of Perl's various quote operators like qw.

        True laziness is hard work
Re: prob w/ gatekeeper subroutine
by MidLifeXis (Monsignor) on Sep 29, 2010 at 16:44 UTC

    This is not directly related to your problem, but may I suggest hashing your passwords?

    if ($hasheduserpass{$euser} eq hashfunction($epass)) { ... }

    Unless you need the unencrypted password elsewhere in the program (may I suggest, then, that you evaluate why you need it), you should not store your user's passwords in the clear. At least in my opinion.

    Update: OTOH, if this is an assignment, as it appears it could be, hashing is probably not appropriate. Remember the above advice (and not just for passwords, but for any information that you only use for validation) once you get beyond classroom assignments.

    --MidLifeXis

      Actually this is an assignment. Thanks for the responses. What is the difference between chop and chomp? I'm not exactly sure what you mean by hashing the passwords. Isn't that what I am doing here? I am putting the user name and password of each user inside the hash. And actually changing the operator to eq from == worked. The problem now is that my subroutine that adds a user and pass to the hash doesn't seem to be working. Here is the code:

      sub adduser { print("Please enter the admin password: "); $epass = <STDIN>; chop($epass); if($epass eq $adminpass) { print("Please enter the user you would like to add: "); $usertoadd = <STDIN>; print("Please enter the password for the user: "); $passtoadd = <STDIN>; $userpass{$usertoadd} = $passtoadd; print("That password has been added to the system\n"); } else { print("Wrong admin password\n"); } } print("Would you like to: \n"); print("1. Add new user\n"); print("2. Change admin password\n"); print("3. Enter the system\n"); $select = <STDIN>; if($select == 1) { &adduser; }

      Also the $adminpass is set to 'root'.

        By "hashing" MidLifeXis means crytopgree-phying your passwords using some algorithm. A similar conceptual-algorithmic method is used in what computer science calls "hash tables" via which perl implements the "hash" datatype in the underlying C code using arrays, since C does not have such a native datatype. But really this is a coincident of nomenclature, and if you have not been asked to do it, don't, it has nothing to do with perl hash syntax. Do:

        use strict; use warnings;