Perl script does not work

Discussion in 'Perl Misc' started by CSUIDL PROGRAMMEr, Apr 29, 2006.

  1. 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";
    }
     
    CSUIDL PROGRAMMEr, Apr 29, 2006
    #1
    1. Advertising

  2. CSUIDL PROGRAMMEr

    Matt Garrish Guest

    "CSUIDL PROGRAMMEr" <> wrote in message
    news:...
    > 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
     
    Matt Garrish, Apr 29, 2006
    #2
    1. Advertising

  3. CSUIDL PROGRAMMEr wrote:
    > 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;
    --------------------------------------^--^

    --
    Gunnar Hjalmarsson
    Email: http://www.gunnar.cc/cgi-bin/contact.pl
     
    Gunnar Hjalmarsson, Apr 29, 2006
    #3
  4. "CSUIDL PROGRAMMEr" <> wrote in
    news::

    > 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 <>
    (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
     
    A. Sinan Unur, Apr 29, 2006
    #4
  5. CSUIDL PROGRAMMEr

    John Bokma Guest

    "A. Sinan Unur" <> wrote:

    > * Use the three argument form of open.
    >
    > unless ( open my $old, '<', $file ) {
    > push @errors, "Failed to open '$file': $!";
    > next FILES;
    > }


    Why?


    --
    John Bokma Freelance software developer
    &
    Experienced Perl programmer: http://castleamber.com/
     
    John Bokma, Apr 29, 2006
    #5
  6. CSUIDL PROGRAMMEr

    John Bokma Guest

    Tim Hammerquist <> wrote:

    > John Bokma <> wrote:
    >> "A. Sinan Unur" <> wrote:
    >> > * Use the three argument form of open.
    >> >
    >> > unless ( open my $old, '<', $file ) {
    >> > push @errors, "Failed to open '$file': $!";
    >> > next FILES;
    >> > }

    >>
    >> Why?

    >
    > Be cause there's no longer MTOWTDI.


    There always is :)

    --
    John Bokma Freelance software developer
    &
    Experienced Perl programmer: http://castleamber.com/
     
    John Bokma, Apr 29, 2006
    #6
  7. Tim Hammerquist <> wrote in
    news::

    > John Bokma <> wrote:
    >> Tim Hammerquist <> wrote:
    >>> John Bokma <> wrote:
    >>>> "A. Sinan Unur" <> wrote:
    >>>> > * Use the three argument form of open.
    >>>> >
    >>>> > unless ( open my $old, '<', $file ) {
    >>>> > push @errors, "Failed to open '$file': $!";
    >>>> > next FILES;
    >>>> > }
    >>>>
    >>>> Why?
    >>>
    >>> Be cause there's no longer MTOWTDI.

    >>
    >> There always is :)

    >
    > 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 <>
    (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
     
    A. Sinan Unur, Apr 29, 2006
    #7
  8. CSUIDL PROGRAMMEr

    John Bokma Guest

    "A. Sinan Unur" <> wrote:

    > 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.

    --
    John Bokma Freelance software developer
    &
    Experienced Perl programmer: http://castleamber.com/
     
    John Bokma, Apr 30, 2006
    #8
  9. CSUIDL PROGRAMMEr

    Bart Lateur Guest

    John Bokma wrote:

    >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"


    --
    Bart.
     
    Bart Lateur, Apr 30, 2006
    #9
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. dpackwood
    Replies:
    3
    Views:
    1,869
  2. Phi!
    Replies:
    1
    Views:
    208
  3. Staale
    Replies:
    16
    Views:
    253
    Tad McClellan
    Sep 30, 2004
  4. Gandu
    Replies:
    9
    Views:
    109
    Tore Aursand
    Nov 22, 2004
  5. Yogi
    Replies:
    1
    Views:
    451
    Peter Makholm
    Sep 13, 2012
Loading...

Share This Page