harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.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 15:40:20 GMT


Gregory Shimansky wrote:
> 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).

Yes, to be consistent.  Because if vm.assert_dialog has anything to do 
with a _dialog_ for _asserts_, we should keep with DRLVM tradition and 
name it something only barely, tenuously similar.

How about

   vm.checker_bannana

?

> 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.
> 

Sure you can't fit "checker" in there?

:)

geir

Mime
View raw message