Skip to Main Content
  • Questions

  • Bug in my Pro*C code (renamed by Tom from Pro*C memory leaking)

Breadcrumb

Question and Answer

Tom Kyte

Thanks for the question, Dhruba.

Asked: January 07, 2005 - 10:45 am UTC

Last updated: January 10, 2005 - 2:08 pm UTC

Version: 8.1.7

Viewed 1000+ times

You Asked

Hi Tom,
Hope you are well.
Following is a piece of code in our billing program .
---------------------------------------------------------------
varchar g_systemdate[8];
......
EXEC SQL SELECT to_char(gnvgen.systemdate,'YYYYMMDD')
INTO :g_systemdate
FROM DUAL;

g_systemdate.arr[g_systemdate.len]='\0';
......
billsummaryfile=(char *)malloc(512);
.......
sprintf(billsummaryfile,"%s/xml/%s/%s/%s_%s_%s.xml",
l_output_dir.arr,
g_systemdate.arr,
g_bill_batch_seq.arr,
l_frrec_account_num.arr,
bill_seq_str.arr,
bill_req_seq_str.arr);

l_billsummaryfile = fopen(billsummaryfile, "w");

if (!(l_billsummaryfile))
{
printf("File Open error - File Name :%s\n",billsummaryfile);
}
------------------------------------------------------------------


Obviously g_systemdate[8] should be g_systemdate[9] - it's a big mistake, I accept.
I have a question though.
This program ran for few years and billed million of customers
and did not fall over .

Once this program was modified and compiled in another environment and while executed it created extra characters at the end of g_systemdate and fallen over.
Now if we run this executable - every time it falls over.

If we run the previous executable it never falls over.

In both cases we use Pro*C/C++: Release 8.1.7.3.0 in Oracle 8.1.7
and gcc .

Is there any explanation of this behaviour ?

thanks,
dhruba.

and Tom said...

<quote>
Obviously g_systemdate[8] should be g_systemdate[9] - it's a big mistake, I
accept.
</quote>

it is not a mistake, you are using the varchar2 type -- which is NOT null terminated. So, YYYYMMDD fits in a varchar2 [8] rather nicely. The mistake you made is that:

g_systemdate.arr[g_systemdate.len]='\0'

is NOT YOUR MEMORY SPACE -- YOU DO NOT HAVE ANY RIGHT TO PUT ANYTHING THERE.

The problem is in your C code, it is relying on a null terminated string but VARCHAR types are NOT null terminated. In other words -- you got EXCESSIVELY lucky in the past and there just happened to be a binary zero in memory after your variable!!


The immediate correction you MUST MAKE to all of your code is:

a) either make darn sure your varchar's are one bigger than they need to be so you can actually stuff the null terminator in there or
b) use the standard C formatting stuff to print out only the characters that you have the right to touch:

sprintf(billsummaryfile,"%.*s/xml/%.*s/%.*s/%.*s_%.*s_%.*s.xml",
l_output_dir.len, l_output_dir.arr,
g_systemdate.len, g_systemdate.arr,
g_bill_batch_seq.len, g_bill_batch_seq.arr,
.......



I do b) regularly. %.*s will expect you to send two things

a) the max number of characters to print
b) the pointer to the string



this has ABSOLUTELY NOTHING to do with the version of Oracle -- it has everything to do with a really bad memory overwrite that is probably all throughout your code!



Rating

  (2 ratings)

Is this answer out of date? If it is, please let us know via a Comment

Comments

Thanks for your comments - though my one question unanswered.

DHRUBA DAS GUPTA, January 08, 2005 - 5:49 pm UTC

Hi Tom,
Thanks a lot for your explanation.I am re-writing the program as part of the next release and doing the correction very carefully as you have suggested.

Still one fact remains very unclear and strange to me :
a.I have executed the previous executable several times
in test environment never fallen over.
b.The other recently compiled executable is falling over everytime in the same environment.
a) was executed million times in production - never fallen over.
a) and b) both have the same bug.
You told I was "excessively lucky" - but I'll be really very happy to get some detailed explanation on why a) never falls over b) always falls over ?

thanks,
dhruba.


Tom Kyte
January 08, 2005 - 5:53 pm UTC

a) so, i flipped a coined 5,000,000 times. it always came up heads. Can I derive from that that it'll come up heads next time?

YOU GO SO LUCKY, period. You wrote a binary zero to a piece of memory you did not own. You go soooooooo lucky.

Luck just ran out.

b) good, you found the bug that has been in your code for a long long time.



a) and b) are different machines, code got loaded differently.


Look -- bottom line, it is very very simple. You have an obvious, serious bug in your code -- nothing more, nothing less. Bugs are evil. We kill bugs. Kill this bug and questions no more.




Another option

DHRUBA DAS GUPTA, January 10, 2005 - 1:23 pm UTC

There’s a third solution to the problem with the varchar:

Forget about varchars all together and declare it as a C string, e.g. char sysdate[9]; (including the NULL terminator!). Then add for ORACLE to treat it as a proper host variable and to add the NULL terminator:
EXEC SQL VAR sysdate IS STRING;

Now ORACLE will deal with it as a C string and we don’t have to worry about the missing NULL terminator assuming we have thought about it when defining the size of the array.

Tom - Would you please let us know your opinion whether this method is preferable to you - if not then why ?


Tom Kyte
January 10, 2005 - 2:08 pm UTC

because I've been using pro*c for too many years and varchar feels right, feels natural, it is what I always used.

more

Connor and Chris don't just spend all day on AskTOM. You can also catch regular content via Connor's blog and Chris's blog. Or if video is more your thing, check out Connor's channel and Chris's channel from their Youtube channels. And of course, keep up to date with AskTOM via the official twitter account.