Hi,
My problem was that I've used code:
l = hb_itemGetNL( pItem );
instead of
if ( HB_IS_NUMERIC( pItem ) )
l = hb_itemGetNL( pItem );
else
l = 0;
hoping that return value is 0, if item type is not numeric. But it is not
true for the dates.
I also do not like it. Such "helper" conversions create more troubles
and force adding additional code to protect against them.
The main problem is compatibility with Clipper .c code.
Yes, but do we have many (or any) developers porting Clipper .c code. I
don't think so. BTW, I remember I've found a few times idea that "C API
compatibility is not goal of this project" on this mailing list.
Here is also yet another problem I do not like very much.
hb_par*() and hb_stor*() functions accept optional parameter as array
index. It means that functions like:
HB_FUNC( MYFUNC )
{
hb_retnd( hb_parnd( 1 ) * 3.14 );
}
can access uninitialized memory when user pass array as 1-st parameter
myfunc( {} )
depending on architecture it may cause unpredictable behavior.
In x86 hb_par*() functions use as optional parameter function
return address or some unused part of stack and then this value
is used as array index and may return some random array item.
Nothing very wrong happens and application still run (though maybe
it makes sth different then expected ;-)) but on some other machines
which have hardware stack frame protection it may even cause GPF.
Wow! This never came to my mind. hb_arrayGet*() functions checks, if
array index is valid, so, no access to uninitialized memory can occur,
but results can be really unpredictable (random array value picked). In
x86 the array index parameter should be in the stack area of local
variables. Here is BCC exploit sample code:
PROC main()
? MyFunc({1, 2, 3, 4})
RETURN
#pragma begindump
#include "hbapi.h"
#define INDEX 2 /* Set array index here! :) */
HB_FUNC( MYFUNC )
{
/* Array forces BCC to avoid optimisation and create stack frame */
int i, arr[ 16 ];
for ( i = 0; i < 16; i++ ) arr[ i ] = INDEX;
hb_retnd( hb_parnd( 1 ) * 3.14 );
}
#pragma enddump
To eliminate it we should always pass additional 0 parameter to
hb_par*() and hb_stor*() functions. But it's also not enough because
the optional parameter is extracted as ULONG (it's not Clipper
compatible where optional parameter is int) which may have different
size then int which is used as default for constant values so this
additional parameter must be casted also to ULONG too.
So, hb_parni(1,1) should not work on big endian machine
sizeof(ULONG)!=sizeof(int) and parameters are passed by stack frame?!
BTW we should
fix in Harbour source code all calls to hb_par*() and hb_stor*()
which uses optional parameter as constant value without any casting.
So the above code to not produce any bad side effects should be change
to:
HB_FUNC( MYFUNC )
{
hb_retnd( hb_parnd( 1, ( ULONG ) 0 ) * 3.14 );
}
Personally I'd prefer to eliminate it.
I'd suggest to add functions which will not use optional parameter
or even better changed existing ones (it will nicely help to locate
code which uses optional parameters during compilation so it can be
easy fixed) and add new Clipper compatible functions updating #define
translations in our extend.api.
I think the better way is to change existing functions and create a new
Clipper compatible. If we are not trying to be Clipper compatible, then
the new functions can be more strict on parameters. I mean:
int hb_parani( int iParam, ULONG ulArrayIndex );
instead of Clipper compatible optional array index.
BTW, I can not find (grep -S "hb_par[c-z]*([^)]*," *.c) any usage of
optional parameter in the core code. They are popular in contribs only
to pass structures as arrays, ex., RECT as {top, left, bottom, right}.
Regards,
Mindaugas
_______________________________________________
Harbour mailing list
Harbour@harbour-project.org
http://lists.harbour-project.org/mailman/listinfo/harbour
