If line matches, print column, else print file name

[Yakup]

Hello everyone. I had used Perl a little bit long time ago and now I'm trying to write a little program, but I have got stuck. I would like to check bunch of kickstart files for a '--hostname=' pattern. If it matches I want to get the collumn right after it (actual hostname). Else it should take the name of the file (stripped of '.ks' suffix) and save all of them to the array. Commented lines should not be taken. This is how I have done it in bash + awk

#!/bin/env bash
declare -a hostlist=$(grep -P '(?<!#)--hostname' *.ks | awk -F'=' '{pr
+int $2}')
declare -a not_predefined_hosts=$(grep -L "\-\-hostname" *.ks | cut -d
+\. -f1 )
declare -a commented=$(grep -l ".*#.*\-\-hostname" *.ks | cut -d\. -f1
+ )

for host in "${not_predefined_hosts[@]}"
do
    hostlist+="$host"
done 

for host in "${commented[@]}"
do
    hostlist+="$host"
done

This works, but when I want to do it with perl, I get multiple issues.

#!/usr/bin/env perl
use strict;
use warnings;
use File::Basename;
my $lab_root = dirname $0;

opendir( DH, $lab_root) or die "Cannot open $lab_root: $!\n";
my @kickstarts = grep ( /\.ks$/, readdir(DH));

my @bsname ;
my $hostname;
for my $kickstart (@kickstarts) {
    my $name = (split /\./)[0], $kickstart;
    open my $fh, $kickstart or die "Cannot open $kickstart: $!";
    while (<$fh>) {
        chomp;
        my @fields = split /=/;
        if ( $fields[0] eq '--hostname') {
            $hostname = $fields[1];
            print "$hostname\n";
            push @bsname , $hostname;
        }else {
            $hostname = $name;
            print "$hostname\n";
            push @bsname , $hostname;
        }
       close $fh;
    }
}

First, I get the warnings

Provisioner]$ ./installer.pl 
Useless use of private variable in void context at ./installer.pl line
+ 13.
Use of uninitialized value $_ in split at ./installer.pl line 13.
Use of uninitialized value $hostname in concatenation (.) or string at
+ ./installer.pl line 24, <$fh> line 1.

readline() on closed filehandle $fh at ./installer.pl line 27.
Use of uninitialized value $_ in split at ./installer.pl line 13.
master.myhost.com
readline() on closed filehandle $fh at ./installer.pl line 27.
Use of uninitialized value $_ in split at ./installer.pl line 13.
Use of uninitialized value $hostname in concatenation (.) or string at
+ ./installer.pl line 24, <$fh> line 1.

readline() on closed filehandle $fh at ./installer.pl line 27.
Use of uninitialized value $_ in split at ./installer.pl line 13.
Use of uninitialized value $fields[0] in string eq at ./installer.pl l
+ine 18, <$fh> line 1.
Use of uninitialized value $hostname in concatenation (.) or string at
+ ./installer.pl line 24, <$fh> line 1.

readline() on closed filehandle $fh at ./installer.pl line 27.

I don't understand why I'm getting "Use of uninitialized value" warnings, when I initialize all variables with "my" beforehand. Also, why does the filehandle "$fh" close before the close statement? And what the "Useless use of private variable in void context" mean? All examples I was able to google on it were very different from my code and didn't help me to understand that.

Second, when the code runs,( with added 'print @bsname;' ) it matches only once (but strangely prints twice). There are multiple kickstart files with "--hostname" directive in it, but it ignores the rest. Also the "else" part doesn't work (none of the file names are matched).

[################### Provisioner]$ ./installer.pl 

master.myhost.com


master.myhost.com[################### Provisioner]$

Can somebody please point out mistake in my code? I'm sure it will be something trivial, but I can't wrap my head around it. Thanks!

[Laurent_R]

Don't close your file handle within the while loop reading it.

I'm getting "Use of uninitialized value" warnings, when I initialize all variables with "my" beforehand.

You got it wrong, "my" is used to declare a variable, but it doesn't initialized the variable. Initializing a variable is to give it an initial value.

If you correct the split (as per GotToBTru's previous answer) and move the closing of the filehandle after the end of the while loop, you'll probably have less of these messages, because many of them seem to come from the fact that you are reading from a filehandle that has been closed too early, so that you don't get anything into the $_ variable and the @fields array.

[Yakup]

Laurent_R, GotToBTru thanks a lot to both of you for pointing out mistakes in my code. I don't know how could I not notice them before. I have followed your advices to edit the code and also edited few other things and now it works as expected.

#!/usr/bin/env perl
use strict;
use warnings;
use File::Basename;
my $lab_root = dirname $0;

opendir( DH, $lab_root) or die "Cannot open $lab_root: $!\n";
my @kickstarts = grep ( /\.ks$/, readdir(DH));

my @bsname ;
my @predefined;
my $hostname;
for my $kickstart (@kickstarts) {
    open my $fh, $kickstart or die "Cannot open $kickstart: $!";

    while (<$fh>) {
        chomp;
        if ( /(?s)^((?!#).)*--hostname=/ ) {
            my @fields = split /[=\s]/;
            $hostname = $fields[1];
            push @bsname , $hostname;
            push @predefined , $kickstart;
        }
    }
    close $fh;
    next if $kickstart ~~ @predefined;

    $hostname = (split /\./, $kickstart )[0];
    push @bsname, $hostname;
}

[haukex]

Hi Yakup,

Glad to hear your code is doing what you want. I do have to admit I found the logic a bit hard to follow, but that isn't necessarily a problem. I would however recommend you run the code through enough test cases, just to make sure.

I just wanted to point out a few potential improvements:

• Using FindBin to locate the directory that the Perl script is located in is much more reliable than dirname $0.
• Using glob is easier than opendir+readdir (with the caveat that glob does things a little bit differently, for example it ignores files whose name begin with a dot by default; see File::Glob for all the details); alternatively there are modules like Path::Class or Path::Tiny to help.
• This is just a stylistic thing, but personally I wouldn't make the regex as complex. I might write something like next if /^\s*#/; if (/--hostname=/) ...
• Smart match (~~) has been placed into experimental status and its implementation might change in the future, so in this case something like List::Util's any is better for now. Or, if all you're using @predefined for is to check whether certain strings have been seen before, a hash would be much better.
• The code (split /\./, $kickstart)[0] will only return the first part of $kickstart up to the first dot anywhere in the string, so it will not do what you want (strip the .ks suffix) for strings like "some.file.name.ks" or "/some.path/some.file.ks". I would recommend a regex, or the all-out solution is File::Basename's fileparse.
• Also, I'd recommend the three-argument form of open, and to limit the scope of your variable declarations to where they are needed.

If I implement those suggestions, I get:

#!/usr/bin/env perl
use strict;
use warnings;
use FindBin;
use File::Basename qw/fileparse/;
use List::Util qw/any/;

my $lab_root = $FindBin::Bin;
my @kickstarts = glob "$lab_root/*.ks";

my @bsname;
my @predefined;
for my $kickstart (@kickstarts) {
    open my $fh, '<', $kickstart or die "Cannot open $kickstart: $!";
    while (<$fh>) {
        chomp;
        next if /^\s*#/;
        if (/--hostname=/) {
            my @fields = split /[=\s]/;
            push @bsname, $fields[1];
            push @predefined, $kickstart;
        }
    }
    close $fh;
    next if any {$kickstart eq $_} @predefined;
    my $hostname = fileparse($kickstart, qr/\.ks$/i);
    push @bsname, $hostname;
}

Note that in this code compared to yours, the filenames now all have their pathnames prefixed. Since I don't know what the rest of your code does, this may or may not be a problem for you. If it is, you could for example use the same fileparse function to get only the filename.

Hope this helps,
-- Hauke D

[GotToBTru]

For starters:

my $name = (split /\./,$kickstart)[0];