Sprintf() causing core dump sporadically on old existing code.

Sun Solaris 2.8 running in 32 bit mode with 6.2 compiler.
I've been running into a problem sporadically in the use of sprintf:

sprintf(char_string, " ... %s", function());

where function() returns a character string

This has been causing core dumps in some places (function return is probably in limbo), often in old code that has been in production for years.  It has been popping up when I make changes, usually completely unrelated, other places in the code.  So far it has always been caught in development, but may become a production issue.

***WHY***
To fix this copy the function return to a character array and pass that in to sprintf.

This has been a rare bug so far, but you may more of a problem and I need to come up with the cause. Any ideas?
rayskeltonAsked:
Who is Participating?
 
grg99Commented:

It's always risky to use sprintf() with a %s format string.... How do you know the string is going to fit in the destination char array?.   Much better to use snprintf().  Also it wouldnt hurt to add some checks that the function() always returns a proper char * that isnt NULL or garbage.  And the function() better not return a pointer to a char array local to function(), as that no longer exists by the time sprintf() gets called.

0
 
_nn_Commented:
>> where function() returns a character string

Depends on where that returned string was allocated. If it's a simple variable local to function() (a char[]) then it's just invalid. If it's a static buffer, then you could investigate multi-threading issues. More informations about that function wouldn't hurt...

Finally, sprintf() is dangerous per se, how do you guaranty that char_string is large enough ? Better use snprintf (if supported, I don't know Solaris compilers)
0
 
rayskeltonAuthor Commented:
The char_string in sprintf(char_string, " ... %s", function()); is always allocated and has been running for years, but just recently became an issue. If you allocate a seperate buffer to hold the result of sprintf(), then copy this buffer into the destination string, everyone is happy.

0
Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

 
_nn_Commented:
Ok, if you don't mind, let us put the issue about sprintf itself by the side. I disagree with you about that C standard function and specially about things that have been "running for years", but we'll come back to it after we've checked what looks more important to me.

Generally speaking, the amount of informations and source code you're revealing is pretty tiny, specially when trying to resolve a "sporadic crash". When I said "More informations about that function wouldn't hurt...", I was talking about the called some_function in

sprintf(char_string, " ... %s", some_function());

Can you please give some more details about some_function() ? What exactly is returned and how ? Again, is that application multi-threaded or not ? When you say :

>> To fix this copy the function return to a character array and pass that in to sprintf.

... it suggests that something wrong is going on within that function. If possible, paste the source code here.

Thanks.
0
 
Kent OlsenData Warehouse Architect / DBACommented:

Hi rayskelton,

You can definitively prove (or disprove) if the sprintf() is the cause of the failure or if something else is clobberring your program and it only shows up at this point.

Redefine char_string as dynamic and make a new variable for holding the value returned by function().  Also create a static variable with your format string.

char *char_string = NULL;
char *function_value;
char *FormatString = ".... %s";

Then change the sprintf() to look like this:

function_value = function ();
char_string = realloc (char_string, strlen(FormatString) + strlen (function_value));
sprintf (char_string, FormatString, function_value);


If the code works, then the string generated by sprintf() was longer than was allocated for char_string.  If the code still fails, the problem lies elsewhere in your program.

I assume that your format string is more complicated that what you indicated so make sure that you realloc() a buffer large enough for the string.  The length generated in my example is sufficiently large for this example because the "%s" substring is counted for determining the length, but will be dropped when the substitution is made -- hence the buffer is long enough for the data and the terminating zero.


Good Luck!
Kent
0
 
dimitryCommented:
It may be important to check _nn_ suggestion. Can you please show use function() code ?
Not the whole code but the part where is return value is allocated...
0
 
Kent OlsenData Warehouse Architect / DBACommented:

Hi Ray,

One more thing to check...  Does function() ALWAYS return a non-null value?  There is no guarantee when dereferencing NULL so you could wind up with an error that results in a core dump.  From my previous example, add 1 line so the call to function() is now these two lines:

function_value = function ();
function_value = function_value ? function_value : "<function returned null>";


Again, Good Luck,
Kent
0
 
guynumber5764Commented:
The two main things that'll cause sprintf(sdst, "%s", ssrc) to crap out are 1) ssrc==NULL and 2) strlen(ssrc) > sizeof(sdst) (usually because ssrc was not terminated).
0
 
rayskeltonAuthor Commented:
Therse are many good suggestions. I will try some of these today and try to get some code samples posted. Thanks to all for suggestions.
0
 
rayskeltonAuthor Commented:
The offending code is listed below. The snprintf() resolves all related core dumps, so I am happy with that solution. The List class is around 12 years old and used in thousands of places through the company, so I want to avoid this if possible. This is the method returned in the sprintf(). This just began around siz months ago and is around the same time we converted this code to 64 bit. We have had several bad behaving occurrences of old code, since the upgrade. ll->data is a node within a linked list.

int kb215_add_reason_codes(List & msg_list, NcfResult & ncf)
{
  List *ll = (List *) &ncf.pls_mssg();
  for(ll->rewind(); ll->data() ; (*ll)++ )
  {
    char temp[80];
    sprintf(temp, (char *)"  %s", (char *)ll->data());
    msg_list.append(ll_strdup(temp));
  }

  return(ERR_OK);
}

0
 
grg99Commented:
The sprintf is being used here as a slow and disaster-prone way to put a space in front of a string.

This is a bit faster and safer:

char temp[ 80 ];

strncpy( temp, "  ", sizeof( temp ) );
strncat( temp, ll->data(), sizeof( temp ) );

-------------

You might also try adding a few if() statements to catch the array overrun condition and log it somewhere.
This little problem may be a symptom of a larger problem somewhere else.  Maybe somehow bad data is getting
into your lists.  It would be nice to track down the source of the problem instead of just patching this hole.



0
 
Kent OlsenData Warehouse Architect / DBACommented:

Hi Ray,

sprintf() may have been the quick and easy way to put a space in front of a string (message), but it sure did cause a headache, huh?  If you malloc() a buffer that's large enough for the new string you should be just fine...

Congratulations on isolating the problem!
Kent


int kb215_add_reason_codes(List & msg_list, NcfResult & ncf)
{
  char *temp = NULL;
  List *ll = (List *) &ncf.pls_mssg();
  for(ll->rewind(); ll->data() ; (*ll)++ )
  {
    temp = (char *) realloc (temp, strlen (ll->data)+1+1);  /*  Add one for the leading space, and one more for the zero terminator  */
    strcpy (temp, " ");
    strcat (temp, (char *)ll->data());
    msg_list.append(ll_strdup(temp));
  }
  free (temp);
  return(ERR_OK);
}
0
 
rayskeltonAuthor Commented:
I agree the Linked list code needs some serious scrutiny and is high on my list.

 Thanks to all.
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.