Charles Jardine wrote: > 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.
I'm not that comfortable with the cast to size_t in dbdimp.c because
then it is later cast back to unsigned long and I'd guess on platforms
where size_t is an unsigned long long the compiler might whine about that.
The best fix would be if there was a reliable format for size_t but I
don't know of one.
However, I don't think we need to get this out of proportion after all
it is only two calls and in both cases the size_t is 0 anyway as the
requested attributes are integers and not strings.
> If the current SVN version works for Martin, I suggest that no
> more needs to be done.
It does work.
> I an sorry to have caused this bother.
>
Didn't cost me much bother and was done for all the right reasons.
I am considerably more bothered about this serious problem:
oci8.c
======
/*line 1897 */
if (DBIS->debug >= 3 || dbd_verbose >= 3 || oci_warn){
char buf[10];
sprintf(buf,"bytes");
if (ftype == ORA_CLOB)
sprintf(buf,"characters");
PerlIO_printf(DBILOGFP,
" OCILobRead %s %s: csform %d (%s), LOBlen
%lu(%s), LongReadLen
%lu(%s), BufLen %lu(%s), Got %lu(%s)\n",
name, oci_status_name(status),
csform,oci_csform_name(csform),
ul_t(loblen),buf ,
ul_t(imp_sth->long_readlen),buf,
ul_t(buflen),buf, ul_t(amtp),buf);
}
That sprintf will always overflow buf as buf is 10 chrs long and does
not have room for the trailing NUL.
The attached patch fixes above and a few other ones I saw.
I suggest we try very hard to get someone with a 64bit platform to try
the next RC.
Martin
--
Martin J. Evans
Easysoft Limited
http://www.easysoft.com
1.dif
Description: video/dv
