snprintf vs sprintf

Posted by Greven on Mon 05 Jan 2004 05:25 AM — 6 posts, 16,393 views.

Greven Canada Mon 05 Jan 2004 05:25 AM #0

Well, I went through and changed all my sprintf's to snprintf's, and have come across a small problem with this line:

				sprintf(buf, "%s%s%s", buf, buf[0] == '\0' ? "" : " ", arg);

Now, when its the sprintf version

				snprintf(buf, MSL, "%s%s%s", buf, buf[0] == '\0' ? "" : " ", arg);

It doesn't like it, and cuts off a good portion of it. Now, I know the problem is with copying buf back into itself, but there has to be a way around it other than this:

				{
				char tmp_buf[MSL]; // Crappy hack fix
				strcpy(tmp_buf, buf);
				snprintf(buf, MSL, "%s%s%s", tmp_buf, tmp_buf[0] == '\0' ? "" : " ", arg);
				}

Can anyone explain it?

David Haley USA Mon 05 Jan 2004 06:17 PM #1

Well, it's probably the inner workings of the function that don't allow that to be possible.

Why are you using snprintf anyways?

Nick Gammon Australia Forum Administrator Mon 05 Jan 2004 07:43 PM #2

I actually think the original one is the crappy one, because you are printing something onto itself. This is likely to lead to implementation-dependent bugs.

In any case, the example you give could be rewritten to make it more readable, and avoid the bug, like this:


if (buf [0] == 0)
   strncpy (buf, arg, MSL);  // empty string, just copy arg
else
   {
   strncat (buf, " ", MSL - strlen (buf) - 1);	// add a space
   strncat (buf, arg, MSL - strlen (buf) - 1);  // and now arg
   }

Your version, BTW, throws away any advantage of doing the snprintf in the first place by doing a strcpy inside the function. You should use strncpy in case the string is too long.

Samson USA Tue 06 Jan 2004 10:40 AM #3

Quote:

Why are you using snprintf anyways?

In answer to this part of the question:

snprintf is an overflow safe version of sprintf. It will only accept up to X number of characters into the string. In this particular case, he was specifying MSL characters. I recently went through and did the same with my codebase.

Of course, you can still overflow it if you specify a limit that's larger than the variable should hold, such as trying to copy MSL when you can only hold MIL, etc.

David Haley USA Tue 06 Jan 2004 03:56 PM #4

No no no... I know what the function does, I'm just wondering why you want to use it in a case like this. Your string won't be greater than MSL - depending on what is already in buf - and it should always be <= MAX_INPUT_LENGTH. So, it just seems like an unnecessary exercise to convert all sprintf to snprintf, unless there is some chance that you're actually overflowing - chance that, if the code is written correctly in the first place, should be next to nil.

Nick Gammon Australia Forum Administrator Tue 06 Jan 2004 08:36 PM #5

He is appending "arg" to "buf" so arg may overflow buf if buf is already MSL bytes.

The problem here is really that you are relying on the arguments being declared correctly, as you have noted Greven. If buf, for instance, is not MSL bytes then the test is useless.

What I would recommend if you *really* want to do a lot of work is to use the STL (Standard Template Library), which has a string class. It is virtually impossible to overflow that, and it is more memory-friendly than declaring buf [MSL] all over the place where MSL might be 32000 bytes, but you only need 10 bytes in a particular case.

Then you can simply append things.

eg.


string newbuf;

if (buf.empty ())
  newbuf = arg;
else
  {
  newbuf = buf;
  newbuf += " ";
  newbuf += arg;
  }

There is a fairly lengthy treatment of STL on this forum in the Programming section.