Page 1 of 1

COM_Parse and multiline comments/DP save compatibility

Posted: Fri Jun 22, 2012 9:15 pm
by mh
Inspired by a thread on Func (clicky), here's a handler for multi-line comments in save games and other files that are parsed by COM_Parse. Pop this after the "skip // comments" section and you can load DarkPlaces save games now.

Code: Select all

	// skip /*..*/ comments
	if (c == '/' && data[1] == '*')
	{
		// comment
		data++;

		while (data[0] && (data[0] != '*' || data[1] != '/'))
			data++;

		if (data[0]) data++;
		if (data[0]) data++;

		goto skipwhite;
	}

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Fri Jun 22, 2012 11:15 pm
by taniwha
Hmm, I find it hard to imagine that this would cause any compatibility issues, so it's probably a good thing to spread through the various engines.

However, I see a minor bug: "/*/" is treated as a complete comment because you skip over only the opening "/" but not the opening "*".

The double increment after the check for "*/" seemed suspicious, but then I realized it's skipping the closing "*/". I'm not 100% certain (~98%), but I think you can get away with "if (data[0]) data += 2;" because the only way to get there is for data[0] to be 0, or for data[0..1] to be "*/".

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Fri Jun 22, 2012 11:21 pm
by mh
I was a bit suspicious of the double data++ myself (it's been a coupla years after all) but I've double-checked against the way DarkPlaces handles this and can see that my code was almost a direct copy & paste from it (which I obviously did and then completely forgot about) so I'll leave everything stand.

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Sat Jun 23, 2012 10:52 am
by taniwha
Oh, it's correct, but the second "if (data[0])" is redundant (the double increment should be merged).

However, the first single data++ is not correct (doesn't matter what darkplaces does). I've tested :)

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Sat Jun 23, 2012 8:27 pm
by mh
I don't disagree. I'm just being wary of the fact that LordHavoc never added a comment explaining his reasons for doing it this way, and am wondering if there was some obscure bug it was intended to fix, or was he just having a brainfart that day? Since the double "if (data[0])" does no harm I'm more inclined to leave it be on account of that.

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Sat Jun 23, 2012 11:17 pm
by taniwha
I'd say the biggest problem with the double "if (data[0])" is it is confusing.

Any comments on that single "data++" before the while loop?

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Sat Jun 23, 2012 11:31 pm
by mh
It definitely seems wrong and unnecessary to me too. I can only guess that LH did it because the "handle quoted strings specially" part also does it - but that's all it is, a guess.

Just for reference, here's the relevant section from Q3A:

Code: Select all

		else if (c == '/' && data[1] == '*')
		{
			data += 2;

			while (*data && (*data != '*' || data[1] != '/'))
			{
				data++;
			}

			if (*data)
			{
				data += 2;
			}
		}
I note the use of "data += 2" at the end instead of the double "if (data[0])", but I also note a "data += 2" at the beginning.

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Sat Jun 23, 2012 11:59 pm
by taniwha
That's very similar to how I implemented it in QF. Differences caused by copying your code and different coding style :)

Code: Select all

    // skip /*..*/ comments
    if (data[0] == '/' && data[1] == '*') {
        data += 2;      // skip over the leading /*
        while (data[0] && (data[0] != '*' && data[1] != '/'))
            data++;
        if (data[0])
            data +=2;   // skip over the trailing */
        goto skipwhite;
    }

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Tue Jan 15, 2013 8:12 pm
by szo
The problem with the QF code posted here is that they would stop at the first '*' within the comment, e.g. something like:

Code: Select all

/*
 * $Header$
 */
... will result in '$' as the first available character, an issue which I hit myself. Correct one would be like the following:

Code: Select all

// skip /*..*/ comments
	if (c == '/' && data[1] == '*')
	{
		data += 2;
		while (*data && !(*data == '*' && data[1] == '/'))
			data++;
		if (*data)
			data += 2;
		goto skipwhite;
	}

Re: COM_Parse and multiline comments/DP save compatibility

Posted: Wed Jan 16, 2013 1:28 am
by taniwha
Yeah, I got the email, too. I'm considering how to write a test case for "make check".