On Fri, 11 Dec 2009, Szak�ts Viktor wrote:
Hi,
> > 1. please do not leave any unnecessary casting like:
> > ( LPCTSTR ) HB_PARSTRDEF( ... )
> > such casting does not give anything (it cannot converts strings
> > between different formats) but can hide very serious bugs like wrong
> > HB_PARSTRDEF() body.
> I know about this, but if I don't cast, MSVC (C++ mode) gives me errors
> on compile. (const WCHAR * cannot be converted to LPCWSTR or something
> like it)
It was enough to tell me about it :)
Last night I committed modification which should resolve it:
2009-12-11 04:13 UTC+0100 Przemyslaw Czerpak (druzus/at/priv.onet.pl)
* harbour/include/hbapicdp.h
* define HB_WCHAR as wchar_t on Windows platforms for compilers which
refuse to make conversions between types using the same base type.
> I didn't reevaluate all of them, although I did serious cleanup
> of parcx usage in the past. I was also thinking to fully drop it
> for winapi calls, as I'm sure Windows won't BSOD or crash by
> passing NULL wherever a pointer is required. That's why I didn't
> add yet any special logic to HB_PARSTRDEF() yet. All I wanted
> to do is to keep this information for now.
Windows API is set of functions implemented on different levels
and some of them can crash so it's necessary to check each of
them. We cannot make any global assumption about validation of
NULL pointers. It's even possible that the behavior is different
in Win9x and WinXP, etc.
> > If you will make mechanical translations all hb_parcx() to HB_PARSTRDEF()
> > then I will have to also verify HB_PARSTRDEF() calls. So please try
> > to already clean it at least for such basic things like:
> > if( HB_ISCHAR( 1 ) )
> > {
> > void * hName;
> > xec->hDLL = LoadLibrary( HB_PARSTRDEF( 1, &hName, NULL ) );
> > hb_strfree( hName );
> > }
> This was just out of scope of these modification. To be honest
> I can't easily see how above could be cleaned. In this case the
> winapi docs doesn't mark the parameter as optional, which to me
> means that NULL is not an accepted input. (okay it won't crash
> if passed, so HB_PARSTRDEF() is almost surely unnecessary)
In this case I wanted to show that this code checks HB_ISCHAR( 1 )
so the parameter cannot be NULL and it's not necessary to use
HB_PARSTRDEF() but we can simply use HB_PARSTR()
> > BTW We can add to strapi.c two very simple functions:
> > const char * hb_strnull( const char * str )
> > {
> > return str ? str : "";
> > }
> > const HB_WCHAR * hb_strnull_u16( const HB_WCHAR * str )
> > {
> > return str ? str : s_szConstStr;
> > }
> > which can be used in HB_PARSTRDEF() macro.
> Yes, this would be the perfect solution. Anyhow I didn't
> bother to add default value to HB_PARSTRDEF() yet, as
> in 99% of cases the default is empty string, only one
> was passed some other value, but it's not a requirement
> for WAPI_*() functions to give such extra features so
> it wasn't crucial.
I think that it's necessary in code like:
hb_retnint( ( HB_PTRDIFF ) LoadLibrary( HB_PARSTRDEF( 1, &hName, NULL ) ) );
or at least this code should be fixed.
> If we're at this topic, I've found a few places which
> couldn't be covered by these macros, It'd be very nice
> if you could take a look at them and suggest the
> cleanest way to resolve them. F.e. there are cases
> when retrieved parameter is assigned to win structure,
I do not see the problem, Can you show me the exact code place
where it was problematic?
> there are cases when win return value is returned
> in array elements (win_prn),
We can add hb_array[SG]etStr*() functions.
You can also use hb_arrayGetItemPtr() and hb_itemPutStrLen*()
but I do not like when users use hb_arrayGetItemPtr() function
so in such case try to at least hide it by macros, i.e.:
#if defined( UNICODE )
#define HB_ARRAYSETSTR( arr, n, str ) \
hb_itemPutStrLenU16( hb_arrayGetItemPtr( arr, n ), \
HB_CDP_ENDIAN_NATIVE, str, lstrlenW( str ) )
#define HB_ARRAYSETSTRLEN( arr, n, str, len ) \
hb_itemPutStrLenU16( hb_arrayGetItemPtr( arr, n ), \
HB_CDP_ENDIAN_NATIVE, str, len )
#else
#define HB_ARRAYSETSTR( arr, n, str ) \
hb_itemPutStrLen( hb_arrayGetItemPtr( arr, n ), \
hb_setGetOSCP(), str, strlen( str ) )
#define HB_ARRAYSETSTRLEN( arr, n, str, len ) \
hb_itemPutStrLen( hb_arrayGetItemPtr( arr, n ), \
hb_setGetOSCP(), str, len )
#endif
Please only note that unlike hb_arraySetC[L]() these macros do not
check array size and cannot be safely used with wrong indexes and
HB_ARRAYSETSTRLEN() uses str twice so you should avoid using complex
expressions passing it. It's a problem until we won't have added
hb_arraySetStr*() functions. When we do that then we simply modify
above macros to use new functions.
> there is one case which needs to
> use the retrieved parameter in two places (win_reg),
I do not see nay problem in win_regc.c but maybe I'm missing sth.
Now HB_TCHAR_*() macros are use only in one function:
HB_FUNC( WIN_REGQUERYVALUEEX )
{
LPTSTR lpKey = HB_TCHAR_CONVTO( hb_parcx( 2 ) );
[...]
HB_TCHAR_FREE( lpKey );
}
and should be simply replaced with:
HB_FUNC( WIN_REGQUERYVALUEEX )
{
void * hKey;
LPCTSTR lpKey = HB_PARSTRDEF( 2, &hKey, NULL );
[...]
hb_strfree( hKey );
}
or better by:
HB_FUNC( WIN_REGQUERYVALUEEX )
{
void * hKey;
LPCTSTR lpKey = HB_PARSTR( 2, &hKey, NULL );
[...]
if( lpKey && RegQueryValueEx( ( HKEY ) hb_parptr( 1 ),
[...]
hb_strfree( hKey );
}
> and some more. Probably some additional macros are
> needed?
Probably yes but without exact real life examples it's hard for me
to guess which ones.
best regards,
Przemek
_______________________________________________
Harbour mailing list (attachment size limit: 40KB)
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour
