Perl script does not work

C

CSUIDL PROGRAMMEr

Folk
I am trying hard to run this script on fedora 3

It changes the following pattern in xml files

<!CDATA[slaaam]]> TO

<slaaam>

But it does not work. Any reasons


The script is a as follows

#!/usr/bin/perl
use strict;
use warnings;
my $start_dir = "/home/amjad/chatTrackBot_v1.5/xml_logs";
chdir($start_dir) or die "Can't chdir $start_dir: $!";
my @files = <*.xml>;
my @errors = ();
foreach my $file ( @files ) {
open(OLD, "<$file") or push @errors, "failed to open $file: $!";
open(TEMP, ">temp.txt") or push @errors, "failed to create temp
file: $!";
while (my $line = <OLD>) {
$line =~ s/<!\[CDATA\[([^\]]*)\]\]>/$1/ig;

print TEMP $line;
}
close OLD;
close TEMP;
rename('temp.txt', $file) or push @errors, "Rename failed: $!";
}
if (@errors) {
print "$_\n" for @errors;
}
else {
print "No errors";
}
 
M

Matt Garrish

CSUIDL PROGRAMMEr said:
Folk
I am trying hard to run this script on fedora 3

It changes the following pattern in xml files

<!CDATA[slaaam]]> TO

<slaaam>

But it does not work. Any reasons


How does it not work? What you want it to do is half the equation. What it's
actually doing is also required for people to have any way of helping you.

The script is a as follows

#!/usr/bin/perl
use strict;
use warnings;
my $start_dir = "/home/amjad/chatTrackBot_v1.5/xml_logs";
chdir($start_dir) or die "Can't chdir $start_dir: $!";
my @files = <*.xml>;
my @errors = ();
foreach my $file ( @files ) {
open(OLD, "<$file") or push @errors, "failed to open $file: $!";
open(TEMP, ">temp.txt") or push @errors, "failed to create temp
file: $!";
while (my $line = <OLD>) {
$line =~ s/<!\[CDATA\[([^\]]*)\]\]>/$1/ig;

You're reading the file line-by-line here, so the assumption is the CDATA
block is all on one line. I wouldn't think that's a safe assumption to make.
You might want to consider using a real XML parser.

Matt
 
G

Gunnar Hjalmarsson

CSUIDL said:
It changes the following pattern in xml files

<!CDATA[slaaam]]> TO

<slaaam>

But it does not work. Any reasons

Yes, the regex doesn't match.
$line =~ s/<!\[CDATA\[([^\]]*)\]\]>/$1/ig;
----------------------^^

Try

$line =~ s/<!CDATA\[([^\]]*)\]\]>/<$1>/ig;
--------------------------------------^--^
 
A

A. Sinan Unur

foreach my $file ( @files ) {

FILES: foreach my $file ( @files ) {
open(OLD, "<$file") or push @errors, "failed to open $file: $!";

It is good that you check for errors, but

* Further down, you still attempt to read from this file. You should
stop processing this file if you cannot open it

* Use lexical filehandles.

* perldoc -q vars

* You don't need the paratheses in the open call as you are using the
lower precedence or rather than ||.

* Use the three argument form of open.

unless ( open my $old, '<', $file ) {
push @errors, "Failed to open '$file': $!";
next FILES;
}
open(TEMP, ">temp.txt") or push @errors, "failed to create temp
file: $!";

Ditto.

These are in addition to comments by others.

Sinan

--
A. Sinan Unur <[email protected]>
(remove .invalid and reverse each component for email address)

comp.lang.perl.misc guidelines on the WWW:
http://augustmail.com/~tadmc/clpmisc/clpmisc_guidelines.html
 
J

John Bokma

A. Sinan Unur said:
* Use the three argument form of open.

unless ( open my $old, '<', $file ) {
push @errors, "Failed to open '$file': $!";
next FILES;
}

Why?
 
A

A. Sinan Unur

No. This is Pyth^W the New Perl. :)

(The following is *not* directed at Mr. Bokma.)
....

To bring this slightly back on topic, I can't find any place
where perlfunc says the 1- or 2-arg form of open() is
deprecated. In fact, it mentions several conveniences offered
by the 1-/2-arg form.

I did not go into detail, because I thought it wasn't as important an
issue as attempting to read from and write to possibly unopened
filehandles.

While '>temp.txt' is quite safe, with "<$file", $file is interpolated.
That interpolation can have unintended effects.

In the OP's case, there is no guarantee on what characters may appear in
the filename. In that particular case, perldoc -f open specifically
recommends the 3-argument form:

<blockquote>
Use 3-argument form to open a file with arbitrary weird
characters in it,

open(FOO, '<', $file);

otherwise it's necessary to protect any leading and trailing
whitespace:

$file =~ s#^(\s)#./$1#;
open(FOO, "< $file\0");

(this may not work on some bizarre filesystems). One should
conscientiously choose between the *magic* and 3-arguments form
of open():
</blockquote>

It can also be easier to read because it separates the mode from the
file name.

If the OP had limited the possible filenames to be processed by
filtering them in some way, I would not have made the recommendation.

Therefore, I find it better to stick with the three argument form unless
there is a specific reason to prefer the two argument form.

Sinan

--
A. Sinan Unur <[email protected]>
(remove .invalid and reverse each component for email address)

comp.lang.perl.misc guidelines on the WWW:
http://augustmail.com/~tadmc/clpmisc/clpmisc_guidelines.html
 
J

John Bokma

A. Sinan Unur said:
While '>temp.txt' is quite safe, with "<$file", $file is interpolated.
That interpolation can have unintended effects.

My bad, I hadn't seen the OPs "<$file". I read it as: use

open my $fh, '<', $filename ...

I prefer:

open my $fh, $filename ...

Unless:
In the OP's case, there is no guarantee on what characters may appear in
the filename. In that particular case, perldoc -f open specifically
recommends the 3-argument form:
[...]

If the OP had limited the possible filenames to be processed by
filtering them in some way, I would not have made the recommendation.

Therefore, I find it better to stick with the three argument form unless
there is a specific reason to prefer the two argument form.

clear, thanks. Should have had a better peek at the OP's post, apologies.
 
B

Bart Lateur

John said:
My bad, I hadn't seen the OPs "<$file". I read it as: use

open my $fh, '<', $filename ...

I prefer:

open my $fh, $filename ...

Which is even less safe than

open my $fh, "<$filename"
 

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

Forum statistics

Threads
473,996
Messages
2,570,238
Members
46,826
Latest member
robinsontor

Latest Threads

Top