14

numCheck is number between 1-1000. This code gives me a segfault only when I collect the results of sprintf in charcheck. If I simply use sprintf without using the results, I don't get a seg fault. What's happening here?

char * numString;
int charcheck = sprintf(numString, "%d", numCheck);
madth3
  • 7,275
  • 12
  • 50
  • 74
syl
  • 247
  • 1
  • 4
  • 8

5 Answers5

16

You need to provide your own memory for sprintf. Also, don't use sprintf, but rather snprintf:

char buf[1000] = {0};

snprintf(buf, 999, ....);

Alternatively you can allocate memory dynamically:

char * buf = new char[BUFSIZE];
snprintf(buf, BUFSIZE-1, ...);
/* ... */
delete[] buf;
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • 1
    It's undefined behaviour. Sometimes undefined behaviour behaves exactly as you expect it, which is arguably the worst case. – Kerrek SB Aug 25 '11 at 02:16
  • 5
    Why do you recommend `snprintf` over `sprintf`? – Kevin May 02 '14 at 00:52
  • 2
    @Kevin: You can't generally know how many characters`sprintf` will write, so you can't generally use it correctly, portably. Best not to make exceptions even when you think you know better, and especially not to set bad examples. – Kerrek SB May 02 '14 at 08:40
3

The pointer given as the first parameter to sprintf is expected to point to a memory location where sprintf should write the formatted string.

In this case you didn't initialize numString to point to some memory you allocated for the formatted string. Since numString isn't initialized it might point anywhere, and in your case trying to write the formatted output to that location results in a segmentation fault.

sth
  • 222,467
  • 53
  • 283
  • 367
2

The first argument to sprintf must point to a valid buffer. You have a char* but it points to garbage.

Change your code to:

char numString[80] = { };
int charcheck = sprintf(numString, "%d", numCheck);

So that numString actually points to a valid buffer (of 80 characters in this example, all elements of which are initialised to 0).

It would also be good to use snprintf so you can pass the size of your buffer to it, which will help prevent buffer overflows:

const int bufsize = 80;
char numString[bufsize] = { };
int charcheck = snprintf(numString, bufsize - 1, "%d", numCheck);

Notice that you subtract one from the buffer size that you pass to snprintf because you don't want it to use the very last slot, which you want to make sure is NULL to denote the end of the string.

Seth Carnegie
  • 73,875
  • 22
  • 181
  • 249
  • Is there any way I can find out the size of the integer I'm passing in characters so I can allocate the correct amount of space instead of using an arbitrary value like 80? – syl Aug 25 '11 at 02:15
  • @user it would probably be more efficient to just allocate the extra space (you can figure out the maximum characters a number can be by the type you're storing it as) because calculating it would require a cascade of `if`s. You just calculate what the largest number your number is smaller than (e.g. if it's smaller than 10 (but always `> 0`, mind you) it's 1 digit, if it's smaller than 100 it's 2 digits, etc). But allocating space on the stack requires constant time (and a very small constant at that) so as long as you don't do something like `char numString[9999999999999]` you should be fine – Seth Carnegie Aug 25 '11 at 02:16
0

The most straightforward thing to do is to use an array as above, e.g., 

char numString[80] = { };

suggested by Seth, Jesus and Kerrek.

I think the last answer from sth is a good explanation: "the first parameter to sprintf is expected to point to a memory location where sprintf should write the formatted string." So apart from using an array of characters, which would force the allocation of memory for the string, you can also use this:

char *numstring = (char*) malloc(80);

This should let you explicitly free the allocated memory when it is no longer needed.

Jeff
  • 1
  • 1
0

You need to allocate space for the result such as

char numString[50];
int charcheck = sprintf(numString, "%d", numCheck);

In your case the interal workings of sprintf are trying to reference NULL which is the default value for a pointer in your case.

Jesus Ramos
  • 22,940
  • 10
  • 58
  • 88