Re: Bug in tracing in DBD::Oracle

On 27/01/10 17:38, Martin Evans wrote:
> Charles Jardine wrote:
>> On 27/01/10 15:52, Martin Evans wrote:
>>> Hi,
>>>
>>> I was asked to enable ora_verbose and send a trace a few days ago.
>>>
>>> I'm getting a segfault with DBD::Oracle when ora_verbose or dbd_verbose
>>> is set to 15 in the connect method call. The stack trace is:

[snip]

>>> and that refers to the following line in dbdimp.c:
>>>
>>> OCINlsEnvironmentVariableGet_log_stat( &ncharsetid,(size_t)  0,
>>> OCI_NLS_NCHARSET_ID, 0, &rsize ,status );
>>>
>>> Oracle defines the second argument as size_t so I guess that cast of 0
>>> to size_t is ok but ocitrace.h then goes on to cast it again to
>>> (unsigned long long) and the format argument has been changed to %llu.
>>> Although these match it segfaults.
>> I am responsible for this change. It was part of a campaign to avoid
>> warnings
>> when compiling on 64-bit gcc platforms. All that is necessary to avoid
>> the compiler warnings is that the format arguments match the casts
>> (subject to integral promotion).
>>
>> I used (unsigned long long) in this case for maximum portability. I
>> couldn't
>> find any standard that said that (size_t) might not be wider than
>> (unsigned long).
>>
>> If my change breaks PerlIO_vprintf, we must back off. Using (unsigned long)
>> and %lu would work on all platforms I use. Using (unsigned int) and %u,
>> would work in this case, but not for all uses of size_t.
>>
>> This is the only place where I used a %llu or %lld, so there is only
>> one place to change.
>>
>> Martin, can you try changing the casts to (unsigned long) and the formats
>> to %lu, and see if this fixes your problem.
> 
> That is what I did in effect (nearly).
> 
> I took the casts of 0 to size_t out of the 2 calls in dbdimp.c and added
> a cast to size_t on the real call to oracle in the macro. Then I change
> the format in the PerlIO_printf to %lu and change the cast to (unsigned
> long). This works for me and I guess it will work without warning for
> you too.
> 
> This isn't exactly what John has in subversion at the moment.

John seems to have corrected my over-zealous cast, and produced
a version which complies without warning and works on both 32-
and 64-bit platforms. Thank you John.

I prefer his version, with the cast to site_t left where it was,
rather than imported into the macro.

If the current SVN version works for Martin, I suggest that no
more needs to be done.

I an sorry to have caused this bother.

-- 
Charles Jardine - Computing Service, University of Cambridge
[email protected]    Tel: +44 1223 334506, Fax: +44 1223 334679