Sprintf for font->draw()

[Ion Dune]

I'm trying to use sprintf to draw strings to the screen with font->draw. Here is the code I use:

Code: Select all

void CEngine::GUIManager::print(const char* s, ...)
{
    va_list va;
    va_start (va, s);
    v_sprintf (s, va);
    va_end (va);
}

void CEngine::GUIManager::v_sprintf (const char* s, va_list va)
{
	c8 str_c [ 32 ];
	sprintf(str_c, s, va);
	core::stringw str_w=str_c;
	print_w(str_w);
}
void CEngine::GUIManager::print_w(core::stringw what)
{
	font->draw(what.c_str(),
	core::rect<s32>(0,line,0,0),
	video::SColor(200,255,255,255));
	line += 8;
}

It compiles fine, but I get a "Run-time check failure #2 - stack around the variable 'str_c' was corrupted." at the closing bracket of v_sprintf. What am I doing wrong?

Thanks for your time.

[ebo]

ROFL ...

Do you actually know what this line means:

c8 str_c [ 32 ];

[Ion Dune]

An array of characters, ie. a string, I thought. I tried to use a c8*, but it gave me an assert when I initialized it to 0, so I figured that meant I had to allocate something.

So please, if what I did was so hilariously incorrect, could you at least tell me what I need to do?

[ebo]

It's not incorrect per se. It's just bad style, old (C) style and buggy. If you use a pointer you have, of course, to allocate some memory ... you cant just start writing at 0x00. When you use a array of size 32 you should NOT write something there thats longer than 32 chars. Ever heard of buffer overflows.

What you _really_ need to do is read a book about programming... learn something about heap, stack and pointers. And (imho) dump the c stuff and use c++ streams. They are much easier and safer to use (and cross-platform by default).

[Ion Dune]

Well thats the thing. I never learned C++: I took a short class on C, then gradually learned things about C++ by reading stuff on this forum and this.

I'm basing my code off of this:

Code: Select all

/* sprintf example */
#include <stdio.h>

int main ()
{
  char buffer [50];
  int n, a=5, b=3;
  n=sprintf (buffer, "%d plus %d is %d", a, b, a+b);
  printf ("[%s] is a %d char long string\n",buffer,n);
  return 0;
}

found here.

I'm assuming what is happening is I'm printing more than 32 chars to the character array. So what would be the proper c++ way of doing this?

[arras]

Thinking in C++ 2nd Edition by Bruce Eckel -Free Electronic Book Volume 1 & Volume 2

[greenya]

i would just replaced

Code: Select all

c8 str_c [ 32 ]; 

with

Code: Select all

c8 str_c [ 10000 ]; 

and never worried about that any more.

[Ion Dune]

Thanks for everyone's help, but I've decided to go with what I understand and use irrlicht strings. Here's what I'm using now, in case anyone wants to know:

Code: Select all

//in guimanager class
core::array<core::stringw> buffer;

Code: Select all

void CEngine::GUIManager::print(core::stringw what)
{
	buffer.push_back(what);
}
void CEngine::GUIManager::out()
{
	for ( u32 t=0 ; t<buffer.size() ; t++ )
	{
		font->draw(buffer[t].c_str(),
		core::rect<s32>(0,line,0,0),
		video::SColor(200,255,255,255));
		line += 8;
	}
}

Usage:

Code: Select all

str = L"Falling? ";
str += curGame->cam->falling;
print(str);

Slightly more syntax, but it works so I'm happy.

@arras: thanks for the link, saved both volumes and I'll refer to those in the future

[ebo]

Code: Select all

c8 str_c [ 10000 ]; 

and never worried about that any more.

Because the number is large doesnt mean it won't happen ... especially as 10000 is not _that_ large. Can anyone say buffer overflow?

@Ion Dune:

_Much_ better solution.

[greenya]

As i understand, author of the thread want to use sprintf() for formatting text. This function indeed very useful, the only one problem is that it is just gets the pointer and writes to it without worrying about buffer size (because it is do not know about the size of it).

So, if you want all the list of solutions i see, here it is:

1. write your own sprintf-like function with using stringc or stringw or any other "modern" type (which can handle sizes correctly == dynamically). -- If you can do that -- that would be the best way.

2. if you are writing for Microsoft Windows CE 5.0, you can use _snprintf() to format text. It takes all the same but also the maximum buffer -- just all that is necessary.

3. if you can use Microsoft specific dialect -- you can use function sprintf_s() -- it is secure version of sprintf(). It also receives the maximum buffer size value.

4. since you cannot easily check if you pre-allocated buffer will be enough, it is not-bad-but-ofcause-very-far-from-ideal practice to use very large buffer (like i wrote == 10000). This is the easiest way to solve the problem. The only thing is necessary to remember is use it every time where you completely sure about the length given into function. For example, i using same functions to do logging of actions in the application -- all calls of the function i do with completely known parameters, like "Initialization..." or "Detected video memory: %u Mb" -- shortly, i do not pass values to this function directly from User, and if i need to do so, i just do next:

Code: Select all

...
string userName; // string == some clever string-storage class, like stringc, stringw or CString, or std::string
ReadUserName(&userName);
if (userName.length() > 100)
{
    ShowErrorToUser("To long name");
    return false;
}
else
{
    writeLog("User with name \"%s\" registered", userName); // this function internally based on sprintf() with critical buffer mass == 10000 :)
    return true;
}
...

That is not some working code, but i hope that shows the aim.


SO,
all usages of unsafe sprintf() is just because of its format-abilities. If you do not need them -- do not use this function (you can always use strncpy() -- with it you can handle buffer overflow), BUT if you want formatting features -- write your own sprintf-like function or use very large buffer and hope and pray

[ebo]

Just use the stl ...
And if you really need formatting use boost.