in reply to onlyone
- open (FROMKID, "-|") or exec ("who", "-m") or system_log ("Can't run who: $!");
You probably want to die here, otherwise both processes (open here does an implicit fork) would continue to run in the event your exec fails.
- Regular expressions like /tty(.*?)\s/ are probably a little more efficient written like /tty(\S+)/.
- if (eof FROMKID) {exit 1;}
Instead of exiting, consider die (or croak with the Carp module, if appropriate), which would give you the opportunity to describe the reason you're aborting the script:
die "File is empty" if eof FROMKID;
You can set $! if you need control over the exit value beforehand.
- $killresult = kill 15, $1;
Your code will be a bit more readable if you were to use, say, 'TERM' instead of 15.
- ... or system_log ("Can't run ps: $!");
While I don't see any immediate problem, since $! probably won't be touched by the user, you don't generally want to call syslog with only a single (well, this second) argument, because it leaves you open to format-based attacks. If you only have a single message that might contain user-supplied data (or anything, even incidental, that could have something that looks like a sprintf-style format), you want to call syslog like this:
syslog('info', '%s', $message);
Which means you'd modify your call to system_log thus: system_log('%s', "Couldn't exec ps: $!"). Again, this doesn't look like a problem here, but you should get into the habit so you don't accidentally leave yourself open in the event your code is vulnerable. Taint-checking under Perl (the -T flag) would spot this problem beforehand (tainted data), I think.
- my $hit_enter = <STDIN>;
If you're just discarding the input anyway, just call <STDIN> in a void context:
<STDIN>;
- I'm curious what you're using the English module for..? You don't seem to be using the "long" versions of any system variables. Kudos for using strict, but I'd be most impressed if your script ran warning-free with the -wT options (even if you don't need to use them in your end product).
Hope this helps!
RE: RE: onlyone
by BoredByPolitics (Scribe) on Oct 31, 2000 at 14:52 UTC
|
Thanks for taking the time to reply :)
- open (FROMKID, "-|") or exec ("who", "-m") or system_log ("Can't run who: $!");
You probably want to die here, otherwise both processes (open here does an implicit fork)
would continue to run in the event your exec fails.
Ah, so the die in the open ... or exec ... or die construct actually applies to
the child, not the entire script? I was trying to make sure that any runtime errors get logged
to the system logger, so I can diagnose problems after the event, hence the exit 1 on the following
line. Is there a better way of trying to achive this?
-
Regular expressions like /tty(.*?)\s/ are probably a little more efficient written like /tty(\S+)/.
I'm slowly starting to get to grips with regexp, however I do find it confusing when perl, tcl, and
sed seem to do things slightly differently!
- $killresult = kill 15, $1;
Your code will be a bit more readable if you were to use, say, 'TERM' instead of 15.
That would be better, I hadn't realised I could do that - is that a product of the English module, or built in?
- ... or system_log ("Can't run ps: $!");
While I don't see any immediate problem, since $! probably won't be touched by the user,
you don't generally want to call syslog with only a single (well, this second) argument,
because it leaves you open to format-based attacks.
Thanks for the heads up - I shall adjust my calls to the system_log subroutine accordingly.
- my $hit_enter = <STDIN>;
If you're just discarding the input anyway, just call <STDIN> in a void context:
<STDIN>;
I keep getting confused over when I can do that, and when I can't - guess it's also because my
other main programming language is a varient on Basic!
-
I'm curious what you're using the English module for..? You don't seem to be using the "long"
versions of any system variables.
Ah, that's left over from when I was creating a load of code to make the child safe in open's, but
then changed my mind and decided to make the entire script safe instead.
Kudos for using strict, but I'd be most impressed if your
script ran warning-free with the -wT options (even if you don't need to use them in your end
product).
I must get into the habit of doing that!
Hope this helps!
Immensely, thanks very much :)
| [reply] [d/l] [select] |
|
my $pid = open(FROMKID, "-|");
# There are now two processes running, one with
# $pid set to the PID of the child, and the other
# with $pid set to 0 (or undefined upon an error).
unless ($pid) {
if (!defined($pid)) {
system_logger("Unable to fork: $!");
die "fork: $!"; # or just exit as you were
}
# note that in this test, $pid is undefined,
# which means we're the child thread, so no
# matter what happens, we must exec or exit/die
unless (exec(...)) {
system_logger("Cannot exec ...: $!");
die "exec ...: $!"; # or just exit
}
}
is (the use of 'TERM' in kill) a product of the English module, or built in?
It's built-in.
Hope this helps. | [reply] [d/l] |
|
|