harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gregory Shimansky <gshiman...@gmail.com>
Subject Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Date Tue, 28 Nov 2006 14:35:28 GMT
Salikh Zakirov wrote:
> Alexey Varlamov wrote:
>> Gregory,
>>
>> A couple of spotted issues in the change:
>> 1) Portablity: code is ia32-specific due to unit32-casted assignments
>> to Registers fields. Why don't use POINTER_SIZE_INT or UDATA instead?
> 
> 
> This is in ia32/nt_exception_filter.cpp, so no portability concerns.
> I agree that we may want to generalize it multiple architectures in the future,
> but it wasn't my concern now.

Agreed. The register names are ia32 specific too, so 32-bitness is not 
the only unportable part of the file.

>> 2) It dropped support for "vm.assert_dialog" switch completely, which
>> is proved to be quite useful for various kinds of automated testing.
>> We even discussed recently that launcher lacks similar feature, and I
>> anticipate other developers will raise complains soon.
> 
> Despite the fact that "vm.assert_dialog" check was removed, the support
> is still there: "vm.assert_dialog" controls the behaviour of the default
> handler.
> 
> However, you are right that now the overall behaviour is determined
> by the launcher, so we need to change launcher correspondingly.

Shall we agree that launcher should parse and handle this property? The 
name doesn't look very good to me (Geir will surely ask to change it).

What about vm.windows.crt.debug.dialog ? AFAIK the property has no 
effect on linux, and the word "assert" doesn't reflect the purpose of 
this property too.

-- 
Gregory


Mime
View raw message