Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people, just like you, are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
Solved

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

Posted on 2003-11-14
13
1,599 Views
Last Modified: 2013-12-14
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?
0
Comment
Question by:rayskelton
  • 4
  • 3
  • 2
  • +3
13 Comments
 
LVL 16

Assisted Solution

by:_nn_
_nn_ earned 250 total points
ID: 9749210
>> 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
 

Author Comment

by:rayskelton
ID: 9749424
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
 
LVL 16

Expert Comment

by:_nn_
ID: 9749688
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
Free Tool: Site Down Detector

Helpful to verify reports of your own downtime, or to double check a downed website you are trying to access.

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.

 
LVL 45

Assisted Solution

by:Kent Olsen
Kent Olsen earned 200 total points
ID: 9749774

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
 
LVL 11

Expert Comment

by:dimitry
ID: 9749858
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
 
LVL 45

Expert Comment

by:Kent Olsen
ID: 9749901

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
 
LVL 3

Expert Comment

by:guynumber5764
ID: 9752357
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
 
LVL 22

Accepted Solution

by:
grg99 earned 50 total points
ID: 9754049

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
 

Author Comment

by:rayskelton
ID: 9763484
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
 

Author Comment

by:rayskelton
ID: 9766863
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
 
LVL 22

Expert Comment

by:grg99
ID: 9767125
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
 
LVL 45

Expert Comment

by:Kent Olsen
ID: 9767166

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
 

Author Comment

by:rayskelton
ID: 9767177
I agree the Linked list code needs some serious scrutiny and is high on my list.

 Thanks to all.
0

Featured Post

Free Tool: ZipGrep

ZipGrep is a utility that can list and search zip (.war, .ear, .jar, etc) archives for text patterns, without the need to extract the archive's contents.

One of a set of tools we're offering as a way to say thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Preface I don't like visual development tools that are supposed to write a program for me. Even if it is Xcode and I can use Interface Builder. Yes, it is a perfect tool and has helped me a lot, mainly, in the beginning, when my programs were small…
Here is a helpful source code for C++ Builder programmers that allows you to manage and manipulate HTML content from C++ code, while also handling HTML events like onclick, onmouseover, ... Some objects defined and used in this source include: …
The goal of this video is to provide viewers with basic examples to understand how to use strings and some functions related to them in the C programming language.
Video by: Grant
The goal of this video is to provide viewers with basic examples to understand and use for-loops in the C programming language.

839 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question