[quake3-bugzilla] [Bug 5075] New: Fix comments in quake3 configs

[bugzilla-daemon at icculus.org]

https://bugzilla.icculus.org/show_bug.cgi?id=5075

           Summary: Fix comments in quake3 configs
           Product: ioquake3
           Version: SVN HEAD
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: minor
          Priority: P3
         Component: Misc
        AssignedTo: zakk at icculus.org
        ReportedBy: q3urt.undead at gmail.com
         QAContact: quake3-bugzilla at icculus.org


Created attachment 2818
  --> https://bugzilla.icculus.org/attachment.cgi?id=2818
Patch to fix comments in configs

The way cmd.c scans through configs is not correct.  It should ignore anything
that is a comment.  However, due to a bug, a semicolon or newline will cause a
comment to end and begin interpreting commands.  This was from quake3 as well
and it is not a new issue.

Attached is a patch to fix this problem.  It adds support for // and multiline
/**/ comments.  I have tested it locally using TKN_DBG and it appears to be
doing the right thing (see below).  Note that since I know they are comments, I
could skip the ExecuteString step and make adjustments to skip over it.  I left
that part in there because a) ExecuteString handles comments already and b) it
makes the diff smaller.

This does not handle nested /**/ comments.  It will properly handle multiple
/**/ comments on the same line when they aren't nested.

You may note in the TKN_DBG output that the trailing slash from a /**/ is not
present.  This is because the existing code NULs the character that it stopped
on for ExecuteString.  Since you can have this:  /*comment*/cmd, I have to
overwrite the second '/' or have a new buffer or teach ExecuteString to not
read past a length, etc.  It seems like a minor issue that's only visible with
developer 1 and TKN_DBG so I left it.

This will cause some invalid configs to be interpreted correctly now.  If you
have a messed up /* ... */ sequence (particularly if you relied on a newline to
end it), this will cause your config to be interpreted differently.

Here's an example of r2075 with and without this patch.  I'm using UrT 4.1.1 to
test this but not any of the UrT changes to ioquake3.  I'm setting the
com_standalone and all of that on the commandline.

Example from the end of my config:

// Shouldn't stop here; echo "but it does"

/* Shouldn't read this; echo "but it does" */

/* This shows that multiline comments
   are not properly handled */

// And then I have parts that are intentionally trying to mess it up;
/* this is not */echo "hi"/* what I had */ echo "there" /* in mind */ // too

// needed for; ut_echo /* here */ // too
echo "Hi 1"
/* needed for; ut_echo */echo "here now"
echo "Hi 2"
/* needed for this;
echo "Hi 3"
too ;;
// should be safe here // too
here;;
;ut_echo
ut_echo
;
ut_echo;
 ; 1 */
echo "Hi 4"


I ran this test on 32bit Linux.  I compiled with this Makefile.local for both
versions:
BUILD_STANDALONE=1
CFLAGS += -DTKN_DBG=1

I then symlinked ~/.foo/q3ut4/{autoexec,q3config}.cfg to my real locations so
it would load my full UrT config.

$ ./ioquake3_r2075_with_patch.i386 +set com_standalone 1 +set fs_game q3ut4
+set developer 1 >& log_patched
$ ./ioquake3_r2075_vanilla.i386 +set com_standalone 1 +set fs_game q3ut4 +set
developer 1 >& log_vanilla

Vanilla:
Cmd_TokenizeString: // Shouldn't stop here
Cmd_TokenizeString:  echo "but it does"
but it does
Cmd_TokenizeString:
Cmd_TokenizeString: /* Shouldn't read this
Cmd_TokenizeString:  echo "but it does" */
but it does */
Cmd_TokenizeString:
Cmd_TokenizeString: /* This shows that multiline comments
Cmd_TokenizeString:    are not properly handled */
Unknown command "are^7"
Cmd_TokenizeString:
Cmd_TokenizeString: // And then I have parts that are intentionally trying to
mess it up
Cmd_TokenizeString:
Cmd_TokenizeString: /* this is not */echo "hi"/* what I had */ echo "there" /*
in mind */ // too
hi echo there
Cmd_TokenizeString:
Cmd_TokenizeString: // needed for
Cmd_TokenizeString:  ut_echo /* here */ // too
Unknown command "ut_echo^7"
Cmd_TokenizeString: echo "Hi 1"
Hi 1
Cmd_TokenizeString: /* needed for
Cmd_TokenizeString:  ut_echo */echo "here now"
Unknown command "ut_echo^7"
Cmd_TokenizeString: echo "Hi 2"
Hi 2
Cmd_TokenizeString: /* needed for this
Cmd_TokenizeString:
Cmd_TokenizeString: echo "Hi 3"
Hi 3
Cmd_TokenizeString: too
Unknown command "too^7"
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString: // should be safe here // too
Cmd_TokenizeString: here
Unknown command "here^7"
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString: ut_echo
Unknown command "ut_echo^7"
Cmd_TokenizeString: ut_echo
Unknown command "ut_echo^7"
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString: ut_echo
Unknown command "ut_echo^7"
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString:  1 */
Unknown command "1^7"
Cmd_TokenizeString: echo "Hi 4"
Hi 4
Cmd_TokenizeString:


Patched:
Cmd_TokenizeString: // Shouldn't stop here; echo "but it does"
Cmd_TokenizeString:
Cmd_TokenizeString: /* Shouldn't read this; echo "but it does" *
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString: /* This shows that multiline comments
   are not properly handled *
Cmd_TokenizeString:
Cmd_TokenizeString:
Cmd_TokenizeString: // And then I have parts that are intentionally trying to
mess it up;
Cmd_TokenizeString: /* this is not *
Cmd_TokenizeString: echo "hi"/* what I had *
hi
Cmd_TokenizeString:  echo "there" /* in mind *
there
Cmd_TokenizeString:  // too
Cmd_TokenizeString:
Cmd_TokenizeString: // needed for; ut_echo /* here */ // too
Cmd_TokenizeString: echo "Hi 1"
Hi 1
Cmd_TokenizeString: /* needed for; ut_echo *
Cmd_TokenizeString: echo "here now"
here now
Cmd_TokenizeString: echo "Hi 2"
Hi 2
Cmd_TokenizeString: /* needed for this;
echo "Hi 3"
too ;;
// should be safe here // too
here;;
;ut_echo
ut_echo
;
ut_echo;
 ; 1 *
Cmd_TokenizeString:
Cmd_TokenizeString: echo "Hi 4"
Hi 4
Cmd_TokenizeString:

-- 
Configure bugmail: https://bugzilla.icculus.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.