while (1) 
{
    my @chars = ( "A" .. "Z", "a" .. "z", 0 .. 9, qw(! @ $ % ^ & *) );
    if ($usedpw ne m/$pw/)
    {
        print "Your unique password is: $pw\n";
    }
}
[download]

1. You'll notice the indentation I used. This helps a ton when trying to read code with control structures (like loops, conditionals, etc.)
2. Read Intro to Perl, aka the Llama book. You are trying to program without learning how to program. This will frustrate you to no end.
3. Ask your teacher these questions, not us. :-)

------
We are the carpenters and bricklayers of the Information Age.

Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

Your actaul problem is indeed a syntax error:

# if ($usedpw ne m/$pw/);
# should be

if( CONDITION ) #no semicolon here
{ # BLOCK
}
[download]

There's also something wrong about your if-condition, as it compares (ne does string-comparison) $usedpw to the result of matching whatever is in $pw on $_. This should confuse you, you probably want to use =~

if( $usedpw =~ m/$pw/ ){ #actaully this inverses yourlogic
    print "Your unique password is: $pw\n";
}
[download]

I can't see the sense of this, no matter if you check for "matches" or "doesn't match".

Finally your loop is an endless loop, a very popular mistake, but it looks like you wanted an endless loop because of while(1).

So, what's this code supposed to do?

while (1) 
{
   my @chars = ( "A" .. "Z", "a" .. "z", 0 .. 9, qw(! @ $ % ^ & *) );
   if ($usedpw ne m/$pw/)
   {
      print "Your unique password is: $pw\n";
   }
}
[download]

jeffa

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

while (1) 
{
my @chars = ( "A" .. "Z", "a" .. "z", 0 .. 9, qw(! @ $ % ^ & *) );
if ($usedpw ne m/$pw/);
{
print "Your unique password is: $pw\n";
}
}
[download]

Well, there are a few problems.

• Work on your indentation. Two braces one on top of the other is a big obstacle to readability.
• Why do you have a ';' after the condition in your if statement? Normally an if statement is of the form

  if ( $foo == 1 )
  {
    $bar++;
  }
  [download]

  There's no semi-colon.
• Use 'ne' when comparing strings, '!=' when comparing numeric values and =~ when comparing patterns.
• Get rid of the 'm' in front of the pattern. It's not needed.
• How do you expect to get out of this endless loop if you aren't changing the thing that you're testing?
• Why aren't you using the array that you've defined inside the loop?
• The array definition doesn't change inside the loop (we call that loop-invariant) -- so you can take it outside the loop.
• How about some comments in the code, explaining what you're trying to do. Is it something to do with generating a password? What are the inputs and outputs?

You're gonna slap yourself ;-)

Leave out the ; after the if, i.e.

  if ($usedpw ne m/$pw/)
  {
    ...
  }
[download]

Incidently, you have an infinite loop with while (1) {...}, and I don't think it's what you want.

Hope this helps, -gjb-

#!/usr/bin/perl

use CGI::Carp 'fatalsToBrowser';
use strict;
use warnings;
use CGI;
my $query = CGI->new;

print $query->header;
my $usedpw = "logfile.txt" ;
my @chars = ( "A" .. "Z", "a" .. "z", 0 .. 9, qw(! @ $ % ^ & *) );
my $pw = join @chars[map{rand @chars} (1..17)];

# opening for reading
open(USEDPW, "< $usedpw") or die $!;
flock USEDPW, 1;
close(USEdPW);


while (1) 
{
my @chars = ( "A" .. "Z", "a" .. "z", 0 .. 9, qw(! @ $ % ^ & *) );
if ($usedpw ne m/$pw/)
{
print "Your unique password is: $pw\n";
}
}

open(USEDPW, "> $usedpw") or die $!;
flock USEDPW, 2;
print USEDPW "$pw\n";
close(USEDPW);
[download]

The program is doing what you told it to do. It is looping forever, because you didn't include any way for the loop to end. And it's printing a null password because you never generate a password. The point of the loop is to generate passwords, and keep generating them, UNTIL one is found which has not been generated before.
Please read this and try to understand.

  # define these outside the loop, please.
  my @chars = ( "A" .. "Z", "a" .. "z", 0 .. 9, qw(! @ $ % ^ & *) );

  # load the previously generated passwords from the file:
  my $usedpw = "logfile.txt" ;
  open USEDPW, "< $usedpw" or die "Error opening $usedpw for reading: 
+$!\n";
  flock USEDPW, 1; # shared lock
  my @used_pw = <USEDPW>;
  close USEDPW;
  chomp @used_pw;
  # and put them in a set (implemented as a hash):
  my %used_pw;
  for ( @used_pw ) { $used_pw{$_}++ }

  my $pw;
  do
  { # create a password:
    $pw = join '', @chars[map{rand @chars} (1..17)];
  }
  while exists $used_pw{$pw};

  # we now have a password which has not been used before, so report i
+t
  print "Your unique password is: $pw\n";
  # and store it in the file.
  open USEDPW, "> $usedpw" or die "Error opening $usedpw for writing: 
+$!\n";
  flock USEDPW, 2; # exclusive lock 
  print USEDPW "$pw\n";
  close USEDPW;
[download]

jdporter
The 6th Rule of Perl Club is -- There is no Rule #6.

Please take the advice of other posters and fix your indentation. It will make your life much easier.

The easiest answer as to why your password doesn't match is because you never read anything from the password file. Inside your loop, $usedpw is still set to 'logfile.txt'. I suspect you want something more like this:

open USEDPW, 'logfile.txt' or die "Cannot read logfile: $!\";

my $seen;
while (<USEDPW>)
{
    chomp;
    $seen++, last if $pw eq $_;
}

print "Your unique password is '$pw'\n" unless $seen;
[download]

I'm not sure that's what you really want, though... and it'd be more efficient to use a DBM file.

if I have to worry about how to indent things properly I'd end up getting more confused.

Not trying to sound belittling or anything, but that's like saying "If I have to worry about using the steering wheel, I'd end up getting more confused". The good monks here know what they are talking about. The indention helps you organize your logic flow. It doesn't matter so much in how you do it, just keep it consistent. Proper indentation steers the eye through the logic flow, making it easier for everyone (including yourself) to follow the logic. Use the force, Luke. :)

Yup, indentation is important. There was a poll about it indentation in April, and the commentary is worth reading.

I have found that good indentation is hugely valuable, not just when checking someone else's code, but also when revisiting your own code. It really cuts down on the "What the hell was I thinking?" factor. If you're using vim as a text editor, the perl syntax module will make this much easier, just add "use syntax" to your .exrc file. If there are more than two words you didn't understand in that last sentence, don't worry. It's a unix thing.

On another topic, what are you doing with this code? If you're generating passwords, the code seems to imply that you have a list of passwords in an unencrypted plaintext file. I cannot begin to explain how dangerous that is. Please tell me that this is a class project that will never be used in the real world.

-Logan
"What do I want? I'm an American. I want more."