Skip to main content

Code review for new code submission from Davy Preuveneers (4/24/2008)

1 reply [Last post]
Anonymous

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Danila Sinopalnikov

Hi Davy,

I think it is okay to just remove "ifeq ($(USE_VS2005), true)" test at
that place.

Thanks,
Danila

Davy Preuveneers wrote:
> Hi Danila,
>
> Indeed you are quite right, I made this modification a while ago to avoid the
> warnings when compiling with eVC4 to target WM2003. I did not notice the top
> visCPP test and assumed my patch worked because I had USE_VS2005 set to false
> but still needed the -D_CRT_SECURE_NO_DEPRECATE flag.
>
> I think I was also a bit confused, because the documentation for the compiler
> section says "Visual Studio 6++". I found it rather weird to have a
> test "ifeq ($(USE_VS2005), true)" test in that section. Even below, you have
> another compiler section "Embedded Visual C++ 3.0" with another USE_VS2005
> test.
>
> Is there a reason why the -D_CRT_SECURE_NO_DEPRECATE flag can only be set when
> USE_VS2005 is true in the visCPP section? If there is such a case, would it
> make sense to also add and use another variable (say USE_EVC4) instead? If
> not, perhaps the "ifeq ($(USE_VS2005), true)" test can be removed?
>
> Best regards,
> Davy
>
>
> On Friday 25 April 2008 10:41:46 Danila Sinopalnikov wrote:
>
>> Hi Davy,
>>
>> regarding the second chunk of CLDC patch:
>>
>> @@ -851,7 +855,10 @@
>> CPP_DEF_FLAGS_release =
>> CPP_DEF_FLAGS_product = -DPRODUCT
>>
>> -ifeq ($(USE_VS2005), true)
>> +ifeq ($(compiler), visCPP)
>> +CPP_DEF_FLAGS += -D_CRT_SECURE_NO_DEPRECATE
>> +endif
>> +ifeq ($(compiler), evc)
>> CPP_DEF_FLAGS += -D_CRT_SECURE_NO_DEPRECATE
>> endif
>>
>>
>> This code is in visCPP compiler section, under ifeq ($(compiler),
>> visCPP), so it seems the first condition will always be true and the
>> second will be false.
>>
>> Danila
>>
>> Hinkmond Wong wrote:
>>
>>> This is a call for external code review of a new code submission from
>>> Davy Preuveneers (4/24/2008).
>>>
>>> His changes include changes to MIDP, Personal Basis/Personal Profile,
>>> and CLDC:
>>>
>>> * The Alert patch has been extended a bit with a new method to play
>>> custom sounds (path is harded coded though), but the current behavior
>>> is to still play the default system sounds. This should provide some
>>> audio feedback if JSR 135 is not there.
>>>
>>> * The AwtPPC and AwtPPCStub patches convert some path definitions with
>>> POSIX2HOST and add new missing method (clearBackground). It also provides
>>> a dummy PPCRobotHelper and related classes and sources to get the
>>> Personal Profile compilation working.
>>>
>>> * The CLDC and MIDP patches currently only add configuration support
>>> for WM2003 (to compile with eVC4).
>>>
>>>
>>> Here are the code diffs (vs. rev. 11040 of the phoneME SVN repo):
>>>
>>> https://phoneme.dev.java.net/nonav/issues/showattachment.cgi/10/Alert.rev
>>> 11040.diff
>>>
>>> https://phoneme.dev.java.net/nonav/issues/showattachment.cgi/11/AwtPPC.re
>>> v11040.diff
>>>
>>> https://phoneme.dev.java.net/nonav/issues/showattachment.cgi/12/AwtPPCStu
>>> bs.rev11040.diff
>>>
>>> https://phoneme.dev.java.net/nonav/issues/showattachment.cgi/13/CLDC.rev1
>>> 1040.diff
>>>
>>> https://phoneme.dev.java.net/nonav/issues/showattachment.cgi/14/MIDP.rev1
>>> 1040.diff
>>>
>>> https://phoneme.dev.java.net/nonav/issues/showattachment.cgi/15/Suite.rev
>>> 11040.diff
>>>
>>>
>>>
>>> Davy, please feel free to add any additional comments to this thread
>>> in the forums.
>>>
>>> Everyone else, please take a look at the above diffs and send in your
>>> comments and/or questions on how they look for committing to our
>>> repository.
>>>
>>>
>>> This code review will be open until Fri. 5/23/2008.
>>>
>>>
>>> Thanks,
>>>
>>> Hinkmond
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: feature-unsubscribe@phoneme.dev.java.net
>>> For additional commands, e-mail: feature-help@phoneme.dev.java.net
>>>
>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: feature-unsubscribe@phoneme.dev.java.net
For additional commands, e-mail: feature-help@phoneme.dev.java.net