2012/5/22 Christopher Michael <[email protected]>:
> On 05/21/12 16:34, Sebastian Dransfeld wrote:
>> On 05/21/2012 08:24 AM, Bluezery wrote:
>>> Hello,
>>>
>>> Some efreet APIs do not check input parameters.  So I add checking by
>>> using EINA_SAFETY_ON_XXX().
>>> ISO/IEC statndards says that "If an argument to a function has an
>>> invalid value, behavior is undefined" . But this is  just for the
>>> primitive functions such as libc.  I think that parameter checking is
>>> needed in at least EFL exported APIs
>
> This I 150% support (Not the use of Eina, but rather the checking) !!
> Exported APIs need to be clean....
>
> EFL needs lots of param checking. Has been missing for a bit of time
> now. I try to add it as I go, but not everyone is supportive. Some do
> see it as a "wasted variable check" ... but I would rather KNOW that the
> stuff I am working with IS valid first, before I start to work with it
> ;) ... If not, then what's left ? A Guess that the variable is valid ???
> No thanks ;)
>
>  to prevent run-time abnormal
>>> behavior.
> Bail out nicely if you can ... ;)
>
>>> EINA_SAFTETY_ON_XXX are better than "if (xxx) return" because it gives
>>> error message and can be maintainable.
>
> But adds some overhead for the logging..... 'if (xxx) return' is easier
> to read then chasing down (in a bunch of source code) some obscure
> macros that nobody knows....and printf is cheap (afaik), easily removed...

Yes, "if" is easier and cheaper than "EINA_SAFETY_XXX".
But I prefer "EINA_SAFETY_XXX".  :) because it can prevent NORMAL
user(like me)'s wrong usage by finding bugs more easily (by logging
overhead).
and logging overhead can be reduced by using disable eina safety check
flags when eina is built.
Anyway, It's a matter of taste. :D   and Thank you for your comments.

>>> Actually these may be needed for some static  internal functions. But
>>> I did not add.
>>>
>>> If this patches would be acceptable, I will add these checking into
>>> other efreet files (efreet_menu, efreet_base, etc.)
>>
>> Patch is fine. Just add to other efreet files and I will commit.

Finally I made a patch. please review this patch again.
If this patch is in, we should fix applications by seeing error
messages originated from this changes such as elementary_config. :)

>> S.
>
> Seb, as long as you approve it then no worries ;) Never got into
> efreet...least not much past the initial fdo support .... but I trust
> Seb. He's been here for a long long LONG time...
>
>
> Cheers,
> dh
>
>
>>
>> ------------------------------------------------------------------------------
>> Live Security Virtual Conference
>> Exclusive live event will cover all the ways today's security and
>> threat landscape has changed and how IT managers can respond. Discussions
>> will include endpoint security, mobile security and the latest in malware
>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> _______________________________________________
>> enlightenment-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> enlightenment-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel



-- 
BRs,
Kim.

Attachment: efreet@add_param_check.diff
Description: Binary data

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to