Michael Blumenkrantz <michael.blumenkra...@gmail.com> writes:

> On Wed, 27 Jun 2012 23:43:23 -0300
> Raphael Kubo da Costa <rak...@freebsd.org> wrote:
>
>> Cedric BAIL <cedric.b...@free.fr> writes:
>>
>> > I personally think that eina_iterator_free like any free function
>> > should just work fine with NULL. I was against at that time, but
>> > others won. So we do have this incoherency where some of our free
>> > function work with NULL and some don't.
>>
>> So what can we do to improve the situation here (if it does need to be
>> improved)? Speaking more generically, what criteria are used to decide
>> that a function should be decorated with EINA_ARG_NONNULL and/or have
>> magic checks performed?
>
> I am hugely in favor of having all _free() and _del() functions take NULL
> arguments without erroring.

OK, here's a proposed patch. I tried to change only _free() and _del()
calls (so _get(), _add() etc still bark at you if you pass NULL to
them). Existing if (!foo) return; statements have not been changed.

OTOH, operations such as eina_list_remove(), eina_list_reverse() and
eina_list_sort() only perform the magic check if the list passed to it
is not NULL, which is inconsistent with similar calls in other
places.

I'm unsure about whether to change the following calls, so for now I
haven't:
 - eina_inarray_flush()
 - eina_inarray_remove()
 - eina_tiler_clear()

 - *_foreach():
   One could argue that passing NULL should be equivalent to an empty
   loop. For example, right now
       eina_iterator_foreach(NULL, ..., ...)
   generates a magic check error message, while
       EINA_ITERATOR_FOREACH(NULL, ...)
   does not.

Index: src/lib/eina_hash.c
===================================================================
--- src/lib/eina_hash.c	(revision 73168)
+++ src/lib/eina_hash.c	(working copy)
@@ -485,13 +485,10 @@
 {
    int key_length, key_hash;
 
+   EINA_SAFETY_ON_NULL_RETURN_VAL(hash, EINA_FALSE);
+   EINA_SAFETY_ON_NULL_RETURN_VAL(key, EINA_FALSE);
    EINA_MAGIC_CHECK_HASH(hash);
-   if (!hash)
-     return EINA_FALSE;
 
-   if (!key)
-     return EINA_FALSE;
-
    if (!hash->buckets)
      return EINA_FALSE;
 
Index: src/lib/eina_accessor.c
===================================================================
--- src/lib/eina_accessor.c	(revision 73168)
+++ src/lib/eina_accessor.c	(working copy)
@@ -95,8 +95,8 @@
 EAPI void
 eina_accessor_free(Eina_Accessor *accessor)
 {
+   EINA_SAFETY_ON_NULL_RETURN(accessor);
    EINA_MAGIC_CHECK_ACCESSOR(accessor);
-   EINA_SAFETY_ON_NULL_RETURN(accessor);
    EINA_SAFETY_ON_NULL_RETURN(accessor->free);
    accessor->free(accessor);
 }
Index: src/lib/eina_inarray.c
===================================================================
--- src/lib/eina_inarray.c	(revision 73168)
+++ src/lib/eina_inarray.c	(working copy)
@@ -355,6 +355,7 @@
 EAPI void
 eina_inarray_free(Eina_Inarray *array)
 {
+   EINA_SAFETY_ON_NULL_RETURN(array);
    EINA_MAGIC_CHECK_INARRAY(array);
    free(array->members);
    free(array);
Index: src/lib/eina_matrixsparse.c
===================================================================
--- src/lib/eina_matrixsparse.c	(revision 73168)
+++ src/lib/eina_matrixsparse.c	(working copy)
@@ -971,6 +971,7 @@
    void *user_data;
 
    Eina_Matrixsparse_Row *r;
+   EINA_SAFETY_ON_NULL_RETURN(m);
    EINA_MAGIC_CHECK_MATRIXSPARSE(m);
 
    free_func = m->free.func;
Index: src/lib/eina_model.c
===================================================================
--- src/lib/eina_model.c	(revision 73168)
+++ src/lib/eina_model.c	(working copy)
@@ -3527,6 +3527,7 @@
 EAPI void
 eina_model_del(Eina_Model *model)
 {
+   EINA_SAFETY_ON_NULL_RETURN(model);
    EINA_MODEL_INSTANCE_CHECK(model);
    _eina_model_del(model);
    _eina_model_unref(model);
Index: src/lib/eina_tiler.c
===================================================================
--- src/lib/eina_tiler.c	(revision 73168)
+++ src/lib/eina_tiler.c	(working copy)
@@ -1127,6 +1127,7 @@
 
 EAPI void eina_tiler_free(Eina_Tiler *t)
 {
+   EINA_SAFETY_ON_NULL_RETURN(t);
    EINA_MAGIC_CHECK_TILER(t);
    _splitter_del(t);
    free(t);
@@ -1165,6 +1166,7 @@
 {
    Eina_Rectangle tmp;
 
+   EINA_SAFETY_ON_NULL_RETURN(t);
    EINA_MAGIC_CHECK_TILER(t);
    if ((r->w <= 0) || (r->h <= 0))
       return;
Index: src/lib/eina_simple_xml_parser.c
===================================================================
--- src/lib/eina_simple_xml_parser.c	(revision 73168)
+++ src/lib/eina_simple_xml_parser.c	(working copy)
@@ -585,6 +585,7 @@
 EAPI void
 eina_simple_xml_attribute_free(Eina_Simple_XML_Attribute *attr)
 {
+   EINA_SAFETY_ON_NULL_RETURN(attr);
    EINA_MAGIC_CHECK_ATTRIBUTE(attr);
 
    if (attr->parent)
@@ -669,6 +670,7 @@
 EAPI void
 eina_simple_xml_node_tag_free(Eina_Simple_XML_Node_Tag *tag)
 {
+   EINA_SAFETY_ON_NULL_RETURN(tag);
    EINA_MAGIC_CHECK_TAG(&tag->base);
    if (tag->base.type != EINA_SIMPLE_XML_NODE_TAG)
      {
@@ -716,6 +718,7 @@
 EAPI void
 eina_simple_xml_node_data_free(Eina_Simple_XML_Node_Data *node)
 {
+   EINA_SAFETY_ON_NULL_RETURN(node);
    EINA_MAGIC_CHECK_DATA(&node->base);
    if (node->base.type != EINA_SIMPLE_XML_NODE_DATA)
      {
@@ -735,6 +738,7 @@
 EAPI void
 eina_simple_xml_node_cdata_free(Eina_Simple_XML_Node_Data *node)
 {
+   EINA_SAFETY_ON_NULL_RETURN(node);
    EINA_MAGIC_CHECK_DATA(&node->base);
    if (node->base.type != EINA_SIMPLE_XML_NODE_CDATA)
      {
@@ -754,6 +758,7 @@
 EAPI void
 eina_simple_xml_node_processing_free(Eina_Simple_XML_Node_Data *node)
 {
+   EINA_SAFETY_ON_NULL_RETURN(node);
    EINA_MAGIC_CHECK_DATA(&node->base);
    if (node->base.type != EINA_SIMPLE_XML_NODE_PROCESSING)
      {
@@ -773,6 +778,7 @@
 EAPI void
 eina_simple_xml_node_doctype_free(Eina_Simple_XML_Node_Data *node)
 {
+   EINA_SAFETY_ON_NULL_RETURN(node);
    EINA_MAGIC_CHECK_DATA(&node->base);
    if (node->base.type != EINA_SIMPLE_XML_NODE_DOCTYPE)
      {
@@ -792,6 +798,7 @@
 EAPI void
 eina_simple_xml_node_comment_free(Eina_Simple_XML_Node_Data *node)
 {
+   EINA_SAFETY_ON_NULL_RETURN(node);
    EINA_MAGIC_CHECK_DATA(&node->base);
    if (node->base.type != EINA_SIMPLE_XML_NODE_COMMENT)
      {
Index: src/lib/eina_iterator.c
===================================================================
--- src/lib/eina_iterator.c	(revision 73168)
+++ src/lib/eina_iterator.c	(working copy)
@@ -95,9 +95,9 @@
 EAPI void
 eina_iterator_free(Eina_Iterator *iterator)
 {
-   EINA_MAGIC_CHECK_ITERATOR(iterator);
    EINA_SAFETY_ON_NULL_RETURN(iterator);
    EINA_SAFETY_ON_NULL_RETURN(iterator->free);
+   EINA_MAGIC_CHECK_ITERATOR(iterator);
    iterator->free(iterator);
 }
 
@@ -113,13 +113,10 @@
 EAPI Eina_Bool
 eina_iterator_next(Eina_Iterator *iterator, void **data)
 {
-   if (!iterator)
-     return EINA_FALSE;
-
-   EINA_MAGIC_CHECK_ITERATOR(iterator);
    EINA_SAFETY_ON_NULL_RETURN_VAL(iterator,       EINA_FALSE);
    EINA_SAFETY_ON_NULL_RETURN_VAL(iterator->next, EINA_FALSE);
    EINA_SAFETY_ON_NULL_RETURN_VAL(data,           EINA_FALSE);
+   EINA_MAGIC_CHECK_ITERATOR(iterator);
    return iterator->next(iterator, 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
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to