On Mon, 01 Dec 2008, Mindaugas Kavaliauskas wrote:

Hi Mindaugas,

> I've check Clipper. It seems _itemGetNL() always return value and does not 
> checks item's type at all. It returns object manager handle for strings, 
> arrays and codeblocks, ds segment offsets (in low word of return value) for 
> references and also half (4 bytes) of 8 byte double value. In our language 
> _itemGetNL() is:
>   LONG hb_itemGetNL( PHB_ITEM pItem )
>   {
>      return ( LONG ) pItem->item.asLong.value;
>   }

Yes, in Clipper all _itemGet*() functions seems to work without any type
verification and current Harbour code tries to emulate the Clipper results.

> _parnl() returns numeric values for long, date and double types and 0 
> otherwise.
> 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.

> I can live with it, but I would support removing Julian date value return 
> from hb_itemGetNL() and hb_parnl() to make Harbour implementation more 
> correct and error resistant. Clipper was using those functions for dates 
> because there were no _itemGetDL() and _pardl() in Clipper. We have special 
> function for the dates, so, there is no need to emulate side effects of 
> Clipper.

The main problem is compatibility with Clipper .c code.
Maybe we should add separate functions which will be more restrictive.
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.
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. 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.

best regards,
Przemek
_______________________________________________
Harbour mailing list
Harbour@harbour-project.org
http://lists.harbour-project.org/mailman/listinfo/harbour

Reply via email to