Lately I am implementing some snippets etc, cause I am stuck on a problem I have no idea what to do about it.

So I implemented mapmakerv2.c
And since it output quite too much info when you use world map terrain tight, especially witch colors, I made it so it go through show_string (the usual pager in Rom derivative)

And there all hell broke lose for a simple reason

char buffer[4*MAX_STRING_LENGTH]; (the buffer in wich you will store the modified text)

thing is, if the text to cut in pieces is larger than that, and your lines parameters high enough you WILL overlap this buffer while parsing d->showstr_point
and write something in memory in the wrong place.

My code til yet that seems to work..(for me)

void show_string(struct descriptor_data *d, char *input) {
    char command[MAX_INPUT_LENGTH];
    one_argument(input, command);

    if (command[0] != '\0') { // I removed going back or refresh, not worth it and obfuscate the code logic
        if (d->showstr_head) {
            free_string( &d->showstr_head);
            d->showstr_head = &str_empty[0];
        }
        d->showstr_point = NULL;
        return;
    }

    //pagelines of 200 is fine if char per line is 5…not if it is 100 (like with mpas and lots of colour)
    //so pagelines will be overridden to a lower pagelines if it happens that it cannot fit in MAX_OUTPUT_BUFFER char
    int pagelines = d->original ? d->original->lines : d->character->lines;
    int line = 0;

    char buffer[strlen(d->showstr_point)+1];
    buffer[0] = '\0';
    char *scan = buffer;
    int lastLineCharNumber = 0, currentTextSize = 0;

    if ( d->showstr_point && d->showstr_point[0] != '\0') {
        do {
            *scan = *d->showstr_point;
            if ( *scan == '\n') {
                //here a fast hack that will try to deal with MAX_OUTPUT_BUFFER without counting
                //what size the next line will do so it may not work every time
                if (((lastLineCharNumber  * pagelines) + currentTextSize) > MAX_OUTPUT_BUFFER) //text send will not fit ProtocolOutput
                    while (pagelines >= 2 && ((lastLineCharNumber * (pagelines = pagelines–))+ currentTextSize) > MAX_OUTPUT_BUFFER);
                lastLineCharNumber = 0;
                d->showstr_point ++;
                scan++;//end of a line, yes we always use \n\r so we should always have \r after this one..
                *scan = '\r'; //yes it is ugly but it works…aint gonna code for cases I do not have and WILL NOT..
                scan++; //(hopefully)
                if ((line ++) >= pagelines)
                    break;
            }
            else
                scan++;
            currentTextSize++;
            lastLineCharNumber++;
            if (d->showstr_point[0] == '\0')
                break;
            d->showstr_point ++;
        } while (d->showstr_point[0] != '\0');
    }

    *scan++ = '\0'; //we end the string
    if (d->snoop_by ) {
        write_to_buffer(d->snoop_by, d->character->charname, 0);
        write_to_buffer(d->snoop_by, "% ", 2);
        write_to_buffer(d->snoop_by, buffer, strlen(buffer));
    }
    write_to_buffer(d, buffer, strlen(buffer));
    if (d->showstr_point[0] == '\0') {
        free_string( &d->showstr_head);
        d->showstr_head = &str_empty[0];
        d->showstr_point = NULL;
    }
}

Write your fix accordingly :either char buffer[strlen(d->showstr_point)+1]; or just tell to fuck off if it is too large to handle.

Oh and Kavir snippet just tells you to fuck off from 8192 char anyway. But in a nice way, and not crashing.

btw, I am looking for a snippet that remove redundant colors in a buffer (ie color a, textx, color a, texty that could very well be color a testxtesty) to limit size of this map. Easy to code but quite boring, hoping someone already made it, cause C string manipulation is not my forte…)

Man was I tired when I wrote the code…

while (pagelines >= 2 && (lastLineCharNumber * pagelines + currentTextSize) > MAX_OUTPUT_BUFFER)
                    pagelines–;

to replace line 31 32

Here's a modified version of my color snippet with color compression for the 32 color codes and the 256 color foreground codes.

/*
	xterm 256 color code parser by Igor van den Hoven

	v1.0 02/11/2009
	v1.4 06/24/2011

	This code is placed in the public domain.
*/

/*
	For xterm 256 colors use: <aaa> to <fff> for RGB foreground colors and
	use: <AAA> to <FFF> for RGB background colors.

	Use: <g00> to <g23> for grayscale foreground colors and use: <G00> to
	<G23> for grayscale background colors.

	With 256 colors disabled colors are converted to ANSI colors.
*/

/*
	For ANSI colors use: <xyz>  with x, y, z being parameters

	Parameter 'x': control code

	0 - Reset all colors and codes to default
	1 - Bold
	2 - Dim
	4 - Underscore
	5 - Blink
	7 - Reverse
	8 - Skip (use previous code)

	Parameter 'y':  Foreground color
	Parameter 'z':  Background color

	0 - Black                  5 - Magenta
	1 - Red                    6 - Cyan
	2 - Green                  7 - White
	3 - Yellow                 8 - Skip
	4 - Blue                   9 - Default
*/

/*
	For 32 color codes use:

	^a - dark azure                 ^A - azure
	^b - dark blue                  ^B - blue
	^c - dark cyan                  ^C - cyan
	^e - dark ebony                 ^E - dark
	^g - dark green                 ^G - green
	^j - dark jade                  ^J - jade
	^l - dark lime                  ^L - lime
	^m - dark magenta               ^M - magenta
	^o - dark orange                ^O - orange
	^p - dark pink                  ^P - pink
	^r - dark red                   ^R - red
	^s - dark silver                ^S - silver
	^t - dark tan                   ^T - tan
	^v - dark violet                ^V - violet
	^w - dark white                 ^W - white
	^y - dark yellow                ^Y - yellow
*/

/*
	256 to 16 color conversion table
*/

#include <stdio.h>
#include <string.h>

char *ansi_color[256] =
{
	"<208>", "<218>", "<228>", "<238>", "<248>", "<258>", "<268>", "<278>",
	"<108>", "<118>", "<128>", "<138>", "<148>", "<158>", "<168>", "<178>",

	"<208>", "<248>", "<248>", "<248>", "<148>", "<148>",
	"<228>", "<268>", "<268>", "<248>", "<148>", "<148>",
	"<228>", "<268>", "<268>", "<268>", "<148>", "<148>",
	"<228>", "<228>", "<268>", "<268>", "<268>", "<168>",
	"<128>", "<128>", "<128>", "<268>", "<168>", "<168>",
	"<128>", "<128>", "<128>", "<168>", "<168>", "<168>",

	"<218>", "<258>", "<258>", "<248>", "<148>", "<148>",
	"<238>", "<108>", "<248>", "<248>", "<148>", "<148>",
	"<238>", "<228>", "<268>", "<268>", "<148>", "<148>",
	"<228>", "<228>", "<268>", "<268>", "<268>", "<168>",
	"<128>", "<128>", "<128>", "<268>", "<168>", "<168>",
	"<128>", "<128>", "<128>", "<168>", "<168>", "<168>",

	"<218>", "<258>", "<258>", "<258>", "<148>", "<148>",
	"<238>", "<218>", "<258>", "<258>", "<148>", "<148>",
	"<238>", "<238>", "<278>", "<248>", "<148>", "<148>",
	"<238>", "<238>", "<228>", "<268>", "<268>", "<148>",
	"<128>", "<128>", "<128>", "<268>", "<168>", "<168>",
	"<128>", "<128>", "<128>", "<128>", "<168>", "<168>",

	"<218>", "<218>", "<258>", "<258>", "<258>", "<158>",
	"<218>", "<218>", "<258>", "<258>", "<258>", "<158>",
	"<238>", "<238>", "<218>", "<258>", "<258>", "<148>",
	"<238>", "<238>", "<238>", "<278>", "<148>", "<148>",
	"<238>", "<238>", "<238>", "<128>", "<168>", "<168>",
	"<138>", "<138>", "<128>", "<128>", "<168>", "<168>",

	"<118>", "<118>", "<118>", "<258>", "<158>", "<158>",
	"<118>", "<118>", "<118>", "<258>", "<158>", "<158>",
	"<118>", "<118>", "<118>", "<258>", "<158>", "<158>",
	"<238>", "<238>", "<238>", "<118>", "<158>", "<158>",
	"<138>", "<138>", "<138>", "<138>", "<178>", "<178>",
	"<138>", "<138>", "<138>", "<138>", "<178>", "<178>",

	"<118>", "<118>", "<118>", "<158>", "<158>", "<158>",
	"<118>", "<118>", "<118>", "<158>", "<158>", "<158>",
	"<118>", "<118>", "<118>", "<118>", "<158>", "<158>",
	"<138>", "<138>", "<118>", "<118>", "<158>", "<158>",
	"<138>", "<138>", "<138>", "<138>", "<178>", "<178>",
	"<138>", "<138>", "<138>", "<138>", "<178>", "<178>",

	"<108>", "<108>", "<108>", "<108>", "<108>", "<108>",
	"<108>", "<108>", "<108>", "<108>", "<108>", "<108>",
	"<278>", "<278>", "<278>", "<278>", "<278>", "<278>",
	"<178>", "<178>", "<178>", "<178>", "<178>", "<178>"
};


//	Make sure that the output buffer remains at least 4 times larger than the user input buffer.

int substitute_color(char *input, char *output, int old, int xterm_256_colors)
{
	char *pti, *pto;
	int new;

	pti = input;
	pto = output;

	while (*pti)
	{
		switch (*pti)
		{
			case '^':
				if (isalpha(pti[1]) && pti[2] == '^' && isalpha(pti[3]))
				{
					pti += 2;
					continue;
				}

				switch (pti[1])
				{
					case 'a':
						pto += substitute_color("<abd>", pto, old, xterm_256_colors);
						break;
					case 'A':
						pto += substitute_color("<acf>", pto, old, xterm_256_colors);
						break;
					case 'b':
						pto += substitute_color("<aad>", pto, old, xterm_256_colors);
						break;
					case 'B':
						pto += substitute_color("<aaf>", pto, old, xterm_256_colors);
						break;
					case 'c':
						pto += substitute_color("<add>", pto, old, xterm_256_colors);
						break;
					case 'C':
						pto += substitute_color("<aff>", pto, old, xterm_256_colors);
						break;
					case 'e':
						pto += substitute_color("<g04>", pto, old, xterm_256_colors);
						break;
					case 'E':
						pto += substitute_color("<bbb>", pto, old, xterm_256_colors);
						break;
					case 'j':
						pto += substitute_color("<adb>", pto, old, xterm_256_colors);
						break;
					case 'J':
						pto += substitute_color("<afc>", pto, old, xterm_256_colors);
						break;
					case 'g':
						pto += substitute_color("<ada>", pto, old, xterm_256_colors);
						break;
					case 'G':
						pto += substitute_color("<afa>", pto, old, xterm_256_colors);
						break;
					case 'l':
						pto += substitute_color("<bda>", pto, old, xterm_256_colors);
						break;
					case 'L':
						pto += substitute_color("<cfa>", pto, old, xterm_256_colors);
						break;
					case 'm':
						pto += substitute_color("<dad>", pto, old, xterm_256_colors);
						break;
					case 'M':
						pto += substitute_color("<faf>", pto, old, xterm_256_colors);
						break;
					case 'o':
						pto += substitute_color("<dba>", pto, old, xterm_256_colors);
						break;
					case 'O':
						pto += substitute_color("<fca>", pto, old, xterm_256_colors);
						break;
					case 'p':
						pto += substitute_color("<dab>", pto, old, xterm_256_colors);
						break;
					case 'P':
						pto += substitute_color("<fac>", pto, old, xterm_256_colors);
						break;
					case 'r':
						pto += substitute_color("<daa>", pto, old, xterm_256_colors);
						break;
					case 'R':
						pto += substitute_color("<faa>", pto, old, xterm_256_colors);
						break;
					case 's':
						pto += substitute_color("<ccc>", pto, old, xterm_256_colors);
						break;
					case 'S':
						pto += substitute_color("<eee>", pto, old, xterm_256_colors);
						break;
					case 't':
						pto += substitute_color("<cba>", pto, old, xterm_256_colors);
						break;
					case 'T':
						pto += substitute_color("<eda>", pto, old, xterm_256_colors);
						break;
					case 'v':
						pto += substitute_color("<bad>", pto, old, xterm_256_colors);
						break;
					case 'V':
						pto += substitute_color("<caf>", pto, old, xterm_256_colors);
						break;
					case 'w':
						pto += substitute_color("<ddd>", pto, old, xterm_256_colors);
						break;
					case 'W':
						pto += substitute_color("<fff>", pto, old, xterm_256_colors);
						break;
					case 'y':
						pto += substitute_color("<dda>", pto, old, xterm_256_colors);
						break;
					case 'Y':
						pto += substitute_color("<ffa>", pto, old, xterm_256_colors);
						break;
					default:
						*pto++ = *pti++;
						continue;
				}

				pti += 2;
				break;

			case '<':
				if (isdigit((int) pti[1]) && isdigit((int) pti[2]) && isdigit((int) pti[3]) && pti[4] == '>')
				{
					if (pti[1] != '8' || pti[2] != '8' || pti[3] != '8')
					{
						pto += sprintf(pto, "\033 != '8')
					{
						pto += sprintf(pto, "\033[");

						switch (pti[1])
						{
							case '2':
								pto += sprintf(pto, "22;");
								break;
							case '8':
								break;
							default:
								pto += sprintf(pto, "%c;", pti[1]);
								break;
						}
						switch (pti[2])
						{
							case '8':
								break;
							default:
								pto += sprintf(pto, "3%c;", pti[2]);
								break;
						}
						switch (pti[3])
						{
							case '8':
								break;
							default:
								pto += sprintf(pto, "4%c;", pti[3]);
								break;
						}
						pto–;
						*pto++ = 'm';
					}
					pti += 5;
				}
				else if (pti[1] >= 'a' && pti[1] <= 'f' && pti[2] >= 'a' && pti[2] <= 'f' && pti[3] >= 'a' && pti[3] <= 'f' && pti[4] == '>')
				{
					new = 16 + (pti[1] - 'a') * 36 + (pti[2] - 'a') * 6 + (pti[3] - 'a');

					if (new != old)
					{
						if (xterm_256_colors)
						{
							pto += sprintf(pto, "\033[38;5;%dm", 16 + (pti[1] - 'a') * 36 + (pti[2] - 'a') * 6 + (pti[3] - 'a'));
						}
						else
						{
							pto += substitute_color(ansi_color[16 + (pti[1] - 'a') * 36 + (pti[2] - 'a') * 6 + (pti[3] - 'a')], pto, old, xterm_256_colors);
						}
					}
					old = new;
					pti += 5;
				}
				else if (pti[1] >= 'A' && pti[1] <= 'F' && pti[2] >= 'A' && pti[2] <= 'F' && pti[3] >= 'A' && pti[3] <= 'F' && pti[4] == '>')
				{
					if (xterm_256_colors)
					{
						pto += sprintf(pto, "\033[48;5;%dm", 16 + (pti[1] - 'A') * 36 + (pti[2] - 'A') * 6 + (pti[3] - 'A'));
					}
					else
					{
						pto += sprintf(pto, "\033[4%dm", (pti[1] && pti[1] >= pti[2] ? pti[1] >= pti[3] : 0) +
								(pti[2] && pti[2] >= pti[1] ? pti[2] >= pti[3] : 0) * 2 + (pti[3] && pti[3] >= pti[2] ? pti[3] >= pti[1] : 0) * 4);
					}
					pti += 5;
				}
				else if (pti[1] == 'g' && (pti[2] - '0') * 10 + (pti[3] - '0') >= 0 && (pti[2] - '0') * 10 + (pti[3] - '0') < 24 && pti[4] == '>')
				{
					new =  232 + (pti[2] - '0') * 10 + (pti[3] - '0');

					if (new != old)
					{
						if (xterm_256_colors)
						{
							pto += sprintf(pto, "\033[38;5;%dm", 232 + (pti[2] - '0') * 10 + (pti[3] - '0'));
						}
						else
						{
							pto += substitute_color(ansi_color[232 + (pti[2] - '0') * 10 + (pti[3] - '0')], pto, old, xterm_256_colors);
						}
					}
					pti += 5;
					old = new;
				}
				else if (pti[1] == 'G' && (pti[2] - '0') * 10 + (pti[3] - '0') >= 0 && (pti[2] - '0') * 10 + (pti[3] - '0') < 24 && pti[4] == '>')
				{
					if (xterm_256_colors)
					{
						pto += sprintf(pto, "\033[48;5;%dm", 232 + (pti[2] - '0') * 10 + (pti[3] - '0'));
					}
					else
					{
						pto += sprintf(pto, "\033[4%dm", ((pti[2] - '0') * 10 + (pti[3] - '0')) / 12 ? 7 : 0);
					}
					pti += 5;
				}
				else
				{
					*pto++ = *pti++;
				}
				break;

			default:
				*pto++ = *pti++;
				break;
		}
	}
	*pto = 0;

	return pto - output;
}
[/code]

Probably best to call it with -1 for old, for example: substitute_color(buf1, buf2, -1, ch->pcdata->color == 256 ? 1 : 0);


Edit by kiasyn: Cut a long line in half so it didnt break the view.

Thx a lot, this kind of stuff is hellish to code, lots and lots of cases. I was sure someone already made it.

Code like that reminds me why I dislike bad C code. :sad:

Some suggestions for improvement:
- push color mapping to data, not strewn about the code
- fix chars to use proper constness
- fix the bug about going over the buffer!! You have no idea if you actually have that many characters for look-ahead
- move character testing to subfunction: don't have statements like if char >= a and char <= f – move that to its own function like ishexletter. That will shorten the ugly and unwieldy if statements considerably.
- let the user pass in an output buffer size so that you can test for overrun while you're writing into it, rather than just hoping it works

Coding cookie of the day:

Trying to carry one gallon of water in a one gallon bucket without spilling any of the content is as tricky and time consuming in programming land as it is in real life. Carrying one gallon of water in a five gallon bucket is a breeze in both.

Are you trying to say that your code is the smallest, most efficient possible solution to this problem? Because frankly that's the only sense I can make of your comment.

Oh, and even if you don't care about poor form and difficult maintenance, your code is buggy. :smile: It will blow up if a color code is not terminated correctly near the end of the string and it goes out of bounds with the look-ahead. Is that because you used a one-gallon bucket instead of 1.2 gallons?

If you enjoy adding error checks to keep potentially full arrays from spilling over.. more power to you.

I guess

else if (pti[1] == 'g' should be replaced with else if (pti[1] == 'g' && isdigit((int) pti[2]) && isdigit((int) pti[3])

and

else if (pti[1] == 'G' should be replaced with else if (pti[1] == 'G' && isdigit((int) pti[2]) && isdigit((int) pti[3])

Thanks for catching that one.

For the interested reader, the problem is that it is not safe to check a string beyond its bounds. So while you might know that str[x] is not zero, if you haven't checked the characters beyond that, then you don't know if you're actually staying in the string. Let's say str[x] is non-zero and str[x+1] is zero. Then, checking str[x+2] means you are (if it's a normal 0-terminated string) going out of bounds – and could potentially cause a segfault.

This isn't really just fun and games; it's preventing exactly the kind of silly bug that the FUSS team has spent so much time getting rid of in stock SMAUG: bugs caused by people being lazy or ignorant.

If you don't really care about code being easy to read, easily maintained, or for that matter just plain correct, then if I may borrow Scandum's phrase… more power to you.

:smile:

Or because they lacked good String librairies in the first place, and thought they could do without while a mud is mainly all about that:String Manipulation.
I am though thankful they did not use regex…regex are humanly unreadable.
And we inherit all the clever optimisation they did in a day and age where puting checks was actually too much time consuming (cpu memory). That is why I suggest not using any DikuBase codebase. Not worth the headache. With the time I spent running it through Valgrind to check for all those problems I could have written a new stable engine from scratch…had I knew back then…now what is done is done, and I hope this pager problem will be the last remaining old bug one I will ever have.
Darien also put out a lot of snippets anyone should implement asap in their mud before doing anything else. They did not detect any bugs for me, as I did most of the work already; but it will help keeping stuff in check (The FileOpen/Close and the Buffer Snippet code)

Oh and we now have so much more powerful tools to code those bugs are now way easier to detect than before (powerful ide with power gdb with powerful cpu to run them fast, fast compile time to test things fasters, and the best tool ever for C : Valgrind)

Where did you get that from?

if (isalpha(pti[1]) && pti[2] == '^' && isalpha(pti[3]))(line 141)

From there I guess. You do not know that you are still in the buffer. A potential problem depending on what the code do after that.
It is a VERY common basic source of problems in Diku mud, iterating over String Buffer and never checking if the char you test is actually still in the buffer, as my OP shows.

And just for the laugh..it crashed as soon as I implemented the snippet as is…(it probably works fine if the buffer is 'cleansed' before, but it cannot be plugged as is without checks)

int main(void)
{
	char string[100] = "a string";

	string[50] = 'h';
	if (string[55] == 'o');

	return 0;
}

Reading "it is not safe to check a string beyond its bounds" I'm under the impression that David Haley thinks the above program would be unsafe. Please correct me if I'm wrong.

I actually wonder what could happen if this code was run in a sandboxing layer that could tell you to fuck off for accessing in a place you could have no right at all to read in for security reason.
I mean you could access this way where a password from another program is read. But some OSES can protect memory so it cannot happen.
So yes you are wrong. Though I dont know the internal on how to protect memory, your program may segfault.

I can't figure out if your willing engagement into pedantry is trolling or serious. Maybe you conveniently missed his "could potentially" qualifier. I didn't see him say that under no circumstances is it safe to do this. But did you just prove it's safe under all circumstances (even the non-contrived ones) to access a string beyond the delimiter without regard to the length of the array? How about just the circumstances he was actually responding to?

No, the example Vigud posted is perfectly valid and safe code.

However Scandum's code doesn't know the size of the buffer, so it can't make the same assumptions.

The words you use, like "pedantry" or "trolling" make me think you are trolling me. In any case, I'm doing this, because I care about C and I care about people who could potentially get something wrong about C from reading that "it is not safe to check a string beyond its bounds", which I believe is plain wrong (but I'm ready to learn and apologize if I'm wrong). Programs written in C would be better if C programmers would have been correcting and learning from each other instead of calling each other trolls or pedants.

But I agree with David Haley on that it could potentially cause a segmentation fault! I've never said that he was wrong about that. What I'm asking for is clarification on what the reason of such segfault would be. Because it's not reading 50th byte from an array of 100 bytes that is unsafe.

No, as that wasn't my intention.

Really, it is so simple to access whatever memory you want without ever being told no ? Somehow I doubted it.
http://en.wikipedia.org/wiki/Memory_prot...

That code is actually fine, it were the 'g' and 'G' checks that didn't properly check for the end of the buffer, and it can be fixed by adding the isdigit checks in my previous post.

while (*pti)
	{
		switch (*pti)
		{
			case '^':
				if (isalpha(pti[1]) && pti[2] == '^' && isalpha(pti[3]))

No it is not fine as you go over pti[1] ^ could very well be the last char in your string. pt[1] could be '\0' pti[2] & pti[3] coudl be pretty much anything outside of your string.