search a file and take action if not present

A

al dav

Hi all.
It has been bothering me how messy the following code is. I want to be able
to look for an entry in a file and do something if it is not there.

Is there a neater way of doing this ?

------------------------------------------------------
open("GROUP", "/etc/group");
@GROUP=<GROUP>;
while($g=shift @GROUP){
if ($g=~/^wheel/){
$foundgrp=1;
print "group present\n";
}
}
if (not defined $foundgrp){
system "/sbin/groupadd wheel"
}
 
G

Gunnar Hjalmarsson

al said:
It has been bothering me how messy the following code is. I want to
be able to look for an entry in a file and do something if it is
not there.

Is there a neater way of doing this ?

open GROUP, '/etc/group' or die $!; # check the return from open
my $foundgrp; # declare that variable (you
# are using strictures...?)
while (<GROUP>) { # the array is redundant
if (/^wheel/) {
$foundgrp = 1;
print "group present\n";
last; # stop loop if found
}
}
close GROUP;
if (not defined $foundgrp) {
system "/sbin/groupadd wheel"
}
 
H

Helgi Briem

Is there a neater way of doing this ?

Neater, perhaps. Better, yes.

open("GROUP", "/etc/group");
@GROUP=<GROUP>;
while($g=shift @GROUP){
if ($g=~/^wheel/){
$foundgrp=1;
print "group present\n";
}
}
if (not defined $foundgrp){
system "/sbin/groupadd wheel"
}

#!/usr/bin/perl

use warnings;
use strict;

my $chk_group = 'wheel';

open GROUP, "/etc/group" or die "Cannot open /etc/group:$!\n";
my $chk_group_exists = 0;
while (<GROUP>)
{
my ($group) = split /:/,$_;
if ($group eq $chk_group) { $chk_group_exists = 1; last; }
}
close GROUP;

if ($chk_group_exists) { print "$chk_group group exists\n"; }
else
{
system ("/sbin/groupadd $chk_group") == 0
or die "Cannot add group $chk_group;$?\n";
}
__END__
 
R

Richard Gration

Hi all.
It has been bothering me how messy the following code is. I want to be
able to look for an entry in a file and do something if it is not there.
Is there a neater way of doing this ?
------------------------------------------------------
open("GROUP", "/etc/group");
@GROUP=<GROUP>;
while($g=shift @GROUP){
if ($g=~/^wheel/){
$foundgrp=1;
print "group present\n";
}
}
if (not defined $foundgrp){
system "/sbin/groupadd wheel"
}
---------------------------------------------------------------- Thanks
Dav.

How about:

use strict;
use warnings;

open (GROUP,'/etc/group') or die "Couldn't open group file: $!\n";
if (grep /^wheel/,<GROUP>) {
print "Found group wheel\n";
} else {
system "/sbin/groupadd wheel";
}

HTH
Rich
 
T

Tad McClellan

al dav said:
open("GROUP", "/etc/group");


You should always, yes *always*, check the return value from open().

@GROUP=<GROUP>;
while($g=shift @GROUP){


You should not read the entire file into memory when you only
need one line at a time, you should read one line at a time instead.

Whitespace is not a scarce resourse, feel free to use as many of
them as you like to make your code easier to read and understand.


while ( my $g = <GROUP> ) {
 
B

Ben Morrow

Helgi Briem said:
Neater, perhaps. Better, yes.

#!/usr/bin/perl

use warnings;
use strict;

my $chk_group = 'wheel';

open GROUP, "/etc/group" or die "Cannot open /etc/group:$!\n";

Use a lexical filehandle.
my $chk_group_exists = 0;
while (<GROUP>)
{
my ($group) = split /:/,$_;

$_ is the default.
if ($group eq $chk_group) { $chk_group_exists = 1; last; }
}
close GROUP;

If you use a lexical FH and put it inside a block there's no need to
close it explicitly.
if ($chk_group_exists) { print "$chk_group group exists\n"; }
else
{
system ("/sbin/groupadd $chk_group") == 0
or die "Cannot add group $chk_group;$?\n";

'==0 or' is equivalent to 'and' where the args are definitely numeric.
You should check for $? == -1 and print $!.
You should probably not put "\n" on the end of die messages.
}
__END__

For a solution that's both neater and better, what's wrong with

my $grp = "wheel";

(getgrnam $grp and print "$grp group exists\n")
or (system "/sbin/groupadd", $grp
and die "groupadd failed to add $grp: " . ($? > 0 ? $? : $!) );

?

Ben
 
J

James Willmore

Hi all.
It has been bothering me how messy the following code is. I want to be able
to look for an entry in a file and do something if it is not there.

Is there a neater way of doing this ?

------------------------------------------------------
open("GROUP", "/etc/group");
@GROUP=<GROUP>;
while($g=shift @GROUP){
if ($g=~/^wheel/){
$foundgrp=1;
print "group present\n";
}
}
if (not defined $foundgrp){
system "/sbin/groupadd wheel"
}
----------------------------------------------------------------

You could try using the 'getgrnam' function - this *may* not work on all
platforms. That's if you're doing *exactly* what you posted (reading the
"/etc/group" file and adding a group if it does not exist).

--worked for me--
#!/usr/bin/perl -w

use strict;

my @groups = qw(wheel whee);

for (@groups){
print "Checking for group $_\n";
if(getgrnam($_)){
print "$_ found ... skipping\n";
}else{
(system '/usr/sbin/groupadd', $_) == 0
or warn "Could not add group $_: $?\n";
}
}
--worked for me--

HTH

--
Jim

Copyright notice: all code written by the author in this post is
released under the GPL. http://www.gnu.org/licenses/gpl.txt
for more information.

a fortune quote ...
A long memory is the most subversive idea in America.
 
C

Chris

Richard said:
How about:

use strict;
use warnings;

open (GROUP,'/etc/group') or die "Couldn't open group file: $!\n";
if (grep /^wheel/,<GROUP>) {
print "Found group wheel\n";
} else {
system "/sbin/groupadd wheel";
}

HTH
Rich

Finally. Amazing that so far you are the only one to suggest the "if
(grep)" solution. Otherwise, why don't we all go back to writing this
kind of stuff in C...?

Of course the grp* functions would do just as well...

Chris
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,579
Members
45,053
Latest member
BrodieSola

Latest Threads

Top