[Gnump3d-devel] files.pm

[Aaron Brown]

I was looking at files.pm (on my system it's
/usr/share/perl5/gnump3d/files.pm) to try and see if I had any ideas
that would help someone on the gnump3d-users list who's having trouble
including one file within another.  Looking at the code I think I see a
couple of potential pitfalls, and I want to mention them here to see if:
1) I'm crazy
2) Someone's noticed this already
3) Someone's fixed this already

I apologize if this doesn't quite follow the normal protocol for such
things, but I don't often have code suggestions to submit so I'm not
sure how exactly to go about it.  Anyway, here goes...

The snippets below are both from the subroutine "readFileWithExpansion",
which a search for "sub readFileWithExpansion" should locate easily enough.

1)
    # Check for magic comments.
    foreach my $line ( @LINES )
    {
        # If we've got a comment ..
        if ( $line =~ /(.*)<!-- (.*) -->(.*)/ )

The code snippet above is the first step in spotting comments which
contain special directives (like Echo, Include, etc.).  (Let's call it
an "action comment" for the time being.)  I worry that the regular
expression is too greedy: If I use a line like this:
<!-- include="one.html" --> <!-- include="two.html" -->
...the results will be unpredictable.  The regex would be better like so:
        $line =~ /(.*?)<!--\s+(.*?)\s+-->(.*)/
The (.*?) employs a non-greedy form of wildcard matching, which should
tell perl to eat up as little as possible until encountering the next
specified series of characters ("-->").  The use of \s instead of
[space] is just a personal habit, never knowing what kind of whitespace
people will use.

The whole thing would then be best put into a loop to catch cases like
my example above, where a single line may contain more than one Action
Comment.


2)
        # If we're including a file make sure we expand that
        # too ..
        my @out = ();
        push @out, $pre;
        push @out, &readFileWithExpansion( $params );
        push @out, $post;
        push @CONTENTS, @out;

This snippet is where the Action Comment processing becomes recursive,
checking each included file for yet more Action Comments.  The problem
is that there is no protection against infinite recursion.  There needs
to be a variable outside this local scope or else another parameter
which records all the filenames already processed (a simple hash of the
names would do) to prevent one from being processed twice, creating a
loop.  Right now, I think I could bring my system to its knees with a
couple of files like this:

[index.html]
<!-- include="otherfile.html" -->

[otherfile.html]
<!-- include="index.html" -->


So, what sayeth the dev community?  Am I off base?  Are these really
possible bugs or have they already been fixed?

 - Aaron

-- 
Aaron Brown :: address@hidden :: www.thebrownproject.com