1

I am having quite a bit of trouble with a Perl script I am writing. I want to compare an element of an array to a variable I have to see if they are true. For some reason I cannot seem to get the comparison operation to work correctly. It will either evaluate at true all the time (even when outputting both strings clearly shows they are not the same), or it will always be false and never evaluate (even if they are the same). I have found an example of just this kind of comparison operation on another website, but when I use it it doesn't work. Am I missing something? Is the variable type I take from the file not a string? (Can't be an integer as far as I can tell as it is an IP address).

$ipaddress = '192.43.2.130'
if ($address[0] == ' ')
{
open (FH, "serverips.txt") or die "Crossroads could not find a list of backend servers";
@address = <FH>;
close(FH);
print $address[0];
print $address[1];
}
for ($i = 0; $i < @address; $i++)
{
print "hello";
        if ($address[$i] eq $ipaddress)
        {print $address[$i];
        $file = "server_$i";
        print "I got here first";
        goto SENDING;}
}
SENDING:
print " I am here";

I am pretty weak in Perl, so forgive me for any rookie mistakes/assumptions I may have made in my very meager bit of code. Thank you for you time.

1
  • Yeeeah...I thought I might get a comment about the goto statement. Is there another way to exit that for loop? I couldn't think of anything immediately. Commented Aug 16, 2011 at 17:03

5 Answers 5

3
if ($address[0] == ' ')
{
open (FH, "serverips.txt") or die "Crossroads could not find a list of backend servers";
@address = <FH>;
close(FH);

You have several issues with this code here. First you should use strict because it would tell you that @address is being used before it's defined and you're also using numeric comparison on a string.

Secondly you aren't creating an array of the address in the file. You need to loop through the lines of the file to add each address:

my @address = ();
while( my $addr = <FH> ) {
     chomp($addr); # removes the newline character 
     push(@address, $addr);
}

However you really don't need to push into an array at all. Just loop through the file and find the IP. Also don't use goto. That's what last is for.

while( my $addr = <FH> ) {
     chomp($addr);
     if( $addr eq $ipaddress ) {
           $file = "server_$i";
           print $addr,"\n";
           print "I got here first"; # not sure what this means
           last; # breaks out of the loop
     }
}
Sign up to request clarification or add additional context in comments.

2 Comments

The "I got here first" thing was just some debugging junk I was using to figure out if I made it in if statement. I was considering running the checks against the content of the file, but I was painfully conscious of consuming even a miniscule amount of system resources since this script might run as often as every three seconds. I am sure everyone in the group would probably wave it off as no concern. Thanks!
@stephen - it doesn't consume any extra resources to do the checks while the file is being read. It has to read it line by line either way. By doing the checks during the read you'll save memory (quite a bit if the file is large because you won't have to read in all of it) and the script will be faster, again noticeably so if the file is large. That said if it's something that you are going to run that often, just use grep on the command line.
1

When you're reading in from a file like that, you should use chomp() when doing a comparison with that line. When you do:

print $address[0];
print $address[1];

The output is on two separate lines, even though you haven't explicitly printed a newline. That's because $address[$i] contains a newline at the end. chomp removes this.

if ($address[$i] eq $ipaddress)

could read

my $currentIP = $address[$i];
chomp($currentIP);
if ($currentIP eq $ipaddress)

Once you're familiar with chomp, you could even use:

chomp(my $currentIP = $address[$i]);
if ($currentIP eq $ipaddress)

Also, please replace the goto with a last statement. That's perl's equivalent of C's break.

Also, from your comment on Jack's answer:

Here's some code you can use for finding how long it's been since a file was modified:

my $secondsSinceUpdate = time() - stat('filename.txt')->mtime;

9 Comments

Consider goto, gone! Thanks for the suggestion on using break.
@Stephen I hope you didn't use break itself! I was merely mentioning that it's analogous to the C command. You should, of course, use perl's last.
if (chomp($address[$i]) eq $ipaddress) does not do what you think. chomp returns the number of elements removed, not the string itself, and so your statement becomes: if ("\n" eq $ipaddress). See the documentation
Herk -takes hands away from keyboard- Of course I would never do that! =P Sorry, I got confused. I will be using the 'last' statement
@TLP @Stephen TLP's absolutely right. Bad mistake on my part. I've updated my answer. It also now demonstrates the use of the my keyword in perl. You should get into the habit of declaring variables before you use them, with the my keyword.
|
0

You probably are having an issue with newlines. Try using chomp($address[$i]).

1 Comment

I'll have to look into how chomp works. It looks like a lot of people are suggesting its use.
0

First of all, please don't use goto. Every time you use goto, the baby Jesus cries while killing a kitten.

Secondly, your code is a bit confusing in that you seem to be populating @address after starting the if($address[0] == '') statement (not to mention that that if should be if($address[0] eq '')).

If you're trying to compare each element of @address with $ipaddress for equality, you can do something like the following

Note: This code assumes that you've populated @address.

my $num_matches=0;
foreach(@address)
{
  $num_matches++ if $_ eq $ipaddress;
}

if($num_matches)
{
  #You've got a match!  Do something.
}
else
{
  #You don't have any matches.  This may or may not be bad.  Do something else.
}

Alternatively, you can use the grep operator to get any and all matches from @address:

my @matches=grep{$_ eq $ipaddress}@address;
if(@matches)
{
  #You've got matches.
}
else
{
  #Sorry, no matches.
}

Finally, if you're using a version of Perl that is 5.10 or higher, you can use the smart match operator (ie ~~):

if($ipaddress~~@address)
{
  #You've got a match!
}
else
{
  #Nope, no matches.
}

8 Comments

The line of code checking first element of the array "if($address[0] eq '')" was to be used so that it wouldn't be run every time the script was called. I am unsure if the value of the array is cleared every time the script is completed or not. I am making the assumption that it is not. This script is part of a larger sendmail program which runs quite frequently - in order to save on the number of instructions being run I wanted to try and populate the array field once and then not do it again. I am sure there is a better way to do it, but I am by no means a professional developer.
Well, if you use strict and use warnings (and you are using the strict and warnings pragmas, aren't you??), then the compiler will let you know if you're using @address before you've defined it.
@Stephen Unless you save the data externally, perl has no way of "storing" its current script session for the next execution. Remember, this is all run in memory. When the script terminates, all of its variables and so on are lost. And I don't think populating an array of a handful of IP addresses is going to hurt performance. In any case, my recommendation would be to try and get it working first, then optimize.
@Jared Good point. Keep trying to see the trees and I miss the forest. I guess if I was terribly concerned with performance still I could use an environment variable (Debian 6.0) as a flag. I will probably just end up comparing against the contents of the file after reading through these suggestions.
I'm still not completely sure what you're trying to do, but if you're just trying to avoid reading the file too often, you can check the last modified date of the IP addresses file. For instance: If it's past a certain threshold, read it and use the Unix system command touch on the file to update the last modified date. Otherwise, skip the file reading.
|
0

When you read from a file like that you include the end-of-line character (generally \n) in each element. Use chomp @address; to get rid of it.

Also, use last; to exit the loop; goto is practically never needed.

Here's a rather idiomatic rewrite of your code. I'm excluding some of your logic that you might need, but isn't clear why:

$ipaddress = '192.43.2.130'
open (FH, "serverips.txt") or die "Crossroads could not find a list of backend servers";
while (<FH>) {                  # loop over the file, using the default input space
    chomp;                      # remove end-of-line
    last if ($_ eq $ipaddress); # a RE could easily be used here also, but keep the exact match
}
close(FH);
$file = "server_$.";            # $. is the line number - it's not necessary to keep track yourself
print "The file is $file\n";

Some people dislike using perl's implicit variables (like $_ and $.) but they're not that hard to keep track of. perldoc perlvar lists all these variables and explains their usage.

Regarding the exact match vs. "RE" (regular expression, or regexp - see perldoc perlre for lots of gory details) -- the syntax for testing a RE against the default input space ($_) is very simple. Instead of

last if ($_ eq $ipaddress);

you could use

last if (/$ipaddress/);

Although treating an ip address as a regular expression (where . has a special meaning) is probably not a good idea.

2 Comments

You might want to replace the abbreviation RE with its full term. The OP mentioned he's not all too familiar with perl, so it's probably best to not assume he knows about regex.
@jared - good point, I added some more clarification around that.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.