On Sat, 07 Feb 2009, Szak�ts Viktor wrote:

Hi Viktor,

> I've a patch ready to address BYTE to UCHAR conversion.
> Would you mind reviewing it, before committing?
> http://www.syenar.hu/harbour/byte_2_uchar_dif.zip
> (size: 26KB)

Yes, sure I've just seen it but it's not what I wanted to make.
The sign of 'char' and 'BYTE' types differes between platforms.
It means that they should be used only for text strings or to mark
some raw memory block.
I suggest to change all 'BYTE *' used as file name in Harbour
FS API and similar functions to 'char *' type to not replicate
the problems which comes from Clipper, f.e. from CL53 header files:
   typedef unsigned char BYTE;
   typedef BYTE far * BYTEP;
   extern void _retc(BYTEP);
   extern void _retclen(char far *, unsigned int);
You may find similar problems also in CL52.
It forces explicit casting in C++ mode if char is signed. Such casting
has bad side effect. It can hide typos or even errors normally easy to
catch at compile time, f.e. pFile wrongly used instead of szFile:
   PHB_ITEM pFile = hb_param( 1, HB_IT_STRING );
   char * szFile = hb_itemGetCPtr( pFile )
   HB_FHANDLE hFile = hb_fsOpen( ( BYTE * ) pFile, ... );

For text strings and raw memory blocks the sign is unimportant.
It can be signed or unsigned without any problem and Harbour
code should work with both version. In the past I cleaned char
usage so now Harbour can be compiled and works also on platforms
where 'char' is unsigned.
The sing of 'char' and 'BYTE' should be for us undefined because
it can be different on different platorms.
I suggest to unify all places which uses text strings like file
names to use 'char *'. It allow us to remove unnecessary casting
(char*)<->(BYTE*) and it should be removed with this modification.
But we use BYTE* type also for arrays of unsigned 8-bits integers,
f.e. PCODE, RDD file structures or functions which makes some
calculations on memory blocks using them as array of 8-bits unsigned
integers like most of check sum functions. Here BYTE should be replaced
by UCHAR at least internally. Now Harbour cannot work on platforms where
BYTE is already defined as signed char, f.e. this HVM code will fail:
         case HB_P_SENDSHORT:
            hb_itemSetNil( hb_stackReturnItem() );
            hb_vmSend( pCode[ w + 1 ] );
because pCode[ w + 1 ] after casting to USHORT will give wrong result
for value greater then 128. F.e. for 255 we will have 65535.
We should fix it. Here simple replacing BYTE by UCHAR in PCODE array
definition is enough though really clean version should make sth like:
         case HB_P_SENDSHORT:
            hb_itemSetNil( hb_stackReturnItem() );
            hb_vmSend( HB_PCODE_MKUCHAR( &pCode[ w + 1 ] ) );
but cleaning code to work with PCODE stored in signed or unsigned
bytes so we can keep it in array of BYTEs needs much more work.
In few places when programmer wants to use single byte as signed 8bit
integer then he should use it as 'signed char' or 'SCHAR' because
he cannot assume that 'char' will be signed or unsigned on destination
platform/C compiler. I fixed all such places in core code when I was
working on Harbour port to mips390 CPUs but I want other developers
remember about it creating new code or updating existing one.

In your modifications you replaced BYTE * used as file name in Harbour
FS API to UCHAR *. File names are text strings and  I want to use
simple char * for them and I want to change BYTE used as synonym
of 8bit unsigned integer to UCHAR.
Sorry my fault, I should be more clear earlier and well explain
the problem.

best regards,
Przemek
_______________________________________________
Harbour mailing list
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour

Reply via email to