CODEWITHSUNDEEP
regexperlpattern-matching
devxvda1
devxvda1

Reputation: 462

Perl: unable to get the correct match from the file

I need help with my script. I am writing a script that will check if the username is still existing in /etc/passwd. I know this can be done on BASH but as much as possible I want to avoid using it, and just focus on writing using Perl instead.

Okay, so my problem is that, my script could not find the right match in my $password_file. I still got the No root found error even though it is still in the file.

Execution of the script.

jpd@skriv ~ $ grep root /etc/passwd
root:x:0:0:root:/root:/bin/bash
jpd@skriv ~ $ ~/Copy/documents/scripts/my_perl/test.pl root
Applying pattern match (m//) to @id will act on scalar(@id) at /home/jpd/Copy/documents/scripts/my_perl/test.pl line 16.
No root found!
jpd@skriv ~ $

Also, why do I always get this "Applying pattern match..." warning?

Here's the code:

#!/usr/bin/perl

use strict;
use warnings;

my $stdin = $ARGV[0];
my $password_file = '/etc/passwd';

open (PWD, $password_file) || die "Error: $!\n";
    my @lines = (<PWD>);
close PWD;

for my $line (@lines) {
    my @list = split /:/, $line;
    my @id = "$list[0]\n";
    if (@id =~ m/$stdin/) {
        die "Match found!\n";
    } else {
        print "No $stdin found!\n";
        exit 0;
    }
}

Thanks in advance! :)

Regards,
sedawkgrep
Perl Newbie

Upvotes: 2

Views: 1055

Answers (3)

John F
John F

Reputation: 99

#!/usr/bin/perl

use strict;
use warnings;

my $stdin = $ARGV[0];
my $password_file = '/etc/passwd';

open (PWD,"<$password_file");
my @lines = <PWD>;

my @varr = grep (m/root/, @lines);

Then check varr array and split it if you need.

Upvotes: 2

Miller
Miller

Reputation: 35198

I have a few things to point out regarding your code:

  • Good job using use strict; and use warnings;. They should be included in EVERY perl script.

  • Pick meaningful variable names.

    $stdin is too generic. $username does a better job of documenting the intent of your script.

  • Concerning your file processing:

    • Include use autodie; anytime you're working with files.

      This pragma will automatically handle error messages, and will give you better information than just "Error: $!\n". Also, if you are wanting to do a manual error messages, be sure to remove the new line from your message or die won't report the line number.

    • Use Lexical file handles and the three argument form of open

      open my $fh, '<', $password_file;

    • Don't load an entire file into memory unless you need to. Instead, use while loop and process the file line by line

  • Concerning your comparison: @id =~ m/$stdin/:

    • Always use a scalar to the left of comparison =~

      The comparison operator binds a scalar to a pattern. Therefore the line @id =~ m/$stdin/ is actually comparing the size of @id to your pattern: "1" =~ m/$stdin/. This is obviously a bug.

    • Be sure to escape the regular expression special characters using quotemeta or \Q...\E:

      $list[0] =~ m/\Q$stdin/

    • Since you actually want a direct equality, don't use a regex at all, but instead use eq

  • You're exiting after only processing the first line of your file.

    In one fork you're dying if you find a match in the first line. In your other fork, you're exiting with the assumption that no other lines are going to match either.

With these changes, I would correct your script to the following:

#!/usr/bin/perl
use strict;
use warnings;
use autodie;

my $username = $ARGV[0];
my $password_file = '/etc/passwd';

open my $fh, '<', $password_file;

while (<$fh>) {
    chomp;
    my @cols = split /:/;
    if ($cols[0] eq $username) {
        die "Match found!\n";
    }
}

print "No $username found!\n";

Upvotes: 3

fugu
fugu

Reputation: 6578

You'd be better off using a hash for key lookups, but with minimal modification this should work:

open my $in, '<', 'in.txt';

my $stdin = $ARGV[0];

while(<$in>){
    chomp;
    my @list = split(/\:/);
    my ($id) = $list[0];
    if ($id eq $stdin) {
        die "Match found\n";
    }
}

Upvotes: 0

Related Questions