Re: [E-devel] E SVN: andreas trunk/e/src/modules/conf_screensaver

[Gustavo S. B. <bar...@pr...>]

On Tue, Apr 14, 2009 at 6:05 PM, Christopher Michael
<cpm...@co...> wrote:
> Gustavo Sverzut Barbieri wrote:
>>
>> On Tue, Apr 14, 2009 at 5:16 PM, Christopher Michael
>> <cpm...@co...> wrote:
>>>
>>> Gustavo Sverzut Barbieri wrote:
>>>>
>>>> On Tue, Apr 14, 2009 at 4:09 PM, Andreas Volz <li...@br...>
>>>> wrote:
>>>>>
>>>>> Am Fri, 10 Apr 2009 22:54:45 -0300 schrieb Gustavo Sverzut Barbieri:
>>>>>
>>>>>> On Fri, Apr 10, 2009 at 7:34 PM, Enlightenment SVN
>>>>>> <no-...@en...> wrote:
>>>>>>
>>>>>>>  - now used Eina_List for storage (I hope I used it correct...)
>>>>>>> +   Eina_List *l = NULL;
>>>>>>> +   Evas_Object *o = NULL;
>>>>>>> +
>>>>>>> +   // delete the list
>>>>>>> +   for (l = xscreensaver_list; l; l = eina_list_next(l))
>>>>>>> +   {
>>>>>>> +      xscreensaver_list = eina_list_remove_list(xscreensaver_list,
>>>>>>> l);
>>>>>>> +   }
>>>>>>> +
>>>>>>
>>>>>> please notice:
>>>>>>
>>>>>> l = NULL is dead assignment, the first thing you do later is to "l =
>>>>>> xscreensaver_list, so l = NULL is useless and will trigger an alert in
>>>>>> llvm/clang.
>>>>>
>>>>> Do you really think this is a "problem" that needs to be fixed? Would
>>>>> be the same here:
>>>>>
>>>>> static void
>>>>> _cb_disable_check_list(void *data, Evas_Object *obj)
>>>>> {
>>>>>  Eina_List *list = (Eina_List*) data;
>>>>>  Eina_List *l = NULL;
>>>>>  Evas_Object *o = NULL;
>>>>>
>>>>>  for (l = list, o = eina_list_data_get(l); l; l = eina_list_next(l),
>>>>>      o = eina_list_data_get(l))
>>>>>
>>>>>  {
>>>>>    e_widget_disabled_set(o, !e_widget_check_checked_get(obj));
>>>>>  }
>>>>> }
>>>>>
>>>>> For sure here you're right, but in general I prefer setting new
>>>>> pointers to NULL if the assignment is not in the next line. If someone
>>>>> else later changes the code otherwise this is a source for potential
>>>>> bugs. But here you're right and I could change it.
>>>>
>>>> you don't need to fix it now, maybe do in one go while fixing
>>>> llvm/clang warnings later. But initializing pointers to NULL or
>>>> variables to 0 is not good, if it was be sure that compilers would do
>>>> that automatically. It's easier to hide bugs with that, you'll make it
>>>> harder to valgrind to help you :-/
>>>>
>>> It should be noted tho that you can also run into cases of "variable may
>>> be
>>> used uninitialized" if you don't...which can also lead to issues.
>>
>> very often these warnings are real errors, so more useful than bad.
>>
> Exactly...so IMO it would be better to leave them initialized to NULL (as
> it's certainly not 'hurting' anything), plus it's safer...regardless of what
> clang says.

Ok, my last email, do what you wish as this is not critical, but it's
not *safer*. This is a false claim. it's bad programing habit and will
make debug with valgrind harder. Just like zeroing pointers and memory
before freeing (you loose track of the problem root).

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: bar...@gm...
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202