On Wed, Dec 19, 2012 at 04:24:13PM +0000, Michael Blumenkrantz wrote:
> On Wed, Dec 19, 2012 at 4:20 PM, Leandro Dorileo
> <dori...@profusion.mobi>wrote:
>
> > Hi Maxime,
> >
> > On Wed, Dec 19, 2012 at 03:07:13PM +0100, Maxime Villard wrote:
> > > Hi,
> > > here is a patch.
> > >
> > > 1. free() already null-checks the passed argument, so it is not
> > > necessary to do 'if(x) free(x)'.
> >
> >
> > Isn't that reasoneable to replace if (x) free(x) by E_FREE(x)?
> >
> > Regards.....
> >
> >
> Hi,
>
> This is never, ever a good idea. You can ask Gustavo why.
Ok, let me rephrase it. The patch is leaving some dangling pointers, I suggested
to use E_FREE() but in fact my concern is about the dangling pointers, I didn't
see the
whole context but the patch so I surely may be missing something.
Regards....
>
>
> >
> > >
> > > 2. Moved some '{free(x); x = NULL}' to E_FREE.
> > >
> > > 3. In e_start_main.c, 'buf' is allocated with the size of the
> > > two arguments + '=', so we don't need to check for lenght;
> > > we should use sprintf().
> > >
> > > thanks
> >
> > > Index: bin/e_order.c
> > > ===================================================================
> > > --- bin/e_order.c (r??vision 81358)
> > > +++ bin/e_order.c (copie de travail)
> > > @@ -31,8 +31,7 @@
> > > {
> > > char buf[PATH_MAX];
> > >
> > > - free(menu_file);
> > > - menu_file = NULL;
> > > + E_FREE(menu_file);
> > > snprintf(buf, sizeof(buf),
> > "/etc/xdg/menus/enlightenment.menu");
> > > if (ecore_file_exists(buf)) menu_file = strdup(buf);
> > > else
> > > @@ -45,7 +44,7 @@
> > > }
> > > }
> > > efreet_menu_file_set(menu_file);
> > > - if (menu_file) free(menu_file);
> > > + free(menu_file);
> > > return 1;
> > > }
> > >
> > > Index: bin/e_backlight.c
> > > ===================================================================
> > > --- bin/e_backlight.c (r??vision 81358)
> > > +++ bin/e_backlight.c (copie de travail)
> > > @@ -308,12 +308,12 @@
> > > x_bl = ecore_x_randr_output_backlight_level_get(root,
> > out[i]);
> > > gotten = EINA_TRUE;
> > > }
> > > - if (name) free(name);
> > > + free(name);
> > > }
> > > if (!gotten)
> > > x_bl = ecore_x_randr_output_backlight_level_get(root, out[0]);
> > > }
> > > - if (out) free(out);
> > > + free(out);
> > > if (x_bl >= 0.0)
> > > {
> > > bl_val = x_bl;
> > > @@ -366,7 +366,7 @@
> > > ecore_x_randr_output_backlight_level_set(root,
> > out[i], val);
> > > }
> > > }
> > > - if (out) free(out);
> > > + free(out);
> > > }
> > > #ifdef HAVE_EEZE
> > > else if (sysmode == MODE_SYS)
> > > Index: bin/e_fm.c
> > > ===================================================================
> > > --- bin/e_fm.c (r??vision 81358)
> > > +++ bin/e_fm.c (copie de travail)
> > > @@ -2877,7 +2877,7 @@
> > > _e_fm2_live_file_del
> > > (obj, ecore_file_file_get(path));
> > > }
> > > - if (evdir) free(evdir);
> > > + free(evdir);
> > > break;
> > >
> > > case E_FM_OP_MONITOR_END: /*mon dir del*/
> > > Index: bin/e_manager.c
> > > ===================================================================
> > > --- bin/e_manager.c (r??vision 81358)
> > > +++ bin/e_manager.c (copie de travail)
> > > @@ -207,8 +207,8 @@
> > > }
> > > else
> > > {
> > > - if (wname) free(wname);
> > > - if (wclass) free(wclass);
> > > + free(wname);
> > > + free(wclass);
> > > continue;
> > > }
> > > }
> > > @@ -239,8 +239,7 @@
> > > }
> > > if (data)
> > > {
> > > - free(data);
> > > - data = NULL;
> > > + E_FREE(data);
> > > continue;
> > > }
> > > ret = ecore_x_window_prop_card32_get(windows[i],
> > > Index: bin/e_start_main.c
> > > ===================================================================
> > > --- bin/e_start_main.c (r??vision 81358)
> > > +++ bin/e_start_main.c (copie de travail)
> > > @@ -38,7 +38,7 @@
> > > size_t size = strlen(var) + 1 + strlen(val) + 1;
> > >
> > > buf = alloca(size);
> > > - snprintf(buf, size, "%s=%s", var, val);
> > > + sprintf(buf, "%s=%s", var, val);
> > > if (getenv(var)) putenv(buf);
> > > else putenv(strdup(buf));
> > > #endif
> > > Index: bin/e_pointer.c
> > > ===================================================================
> > > --- bin/e_pointer.c (r??vision 81358)
> > > +++ bin/e_pointer.c (copie de travail)
> > > @@ -308,7 +308,7 @@
> > > if (p->pointer_object) evas_object_del(p->pointer_object);
> > > if (p->hot_object) evas_object_del(p->hot_object);
> > > if (p->evas) evas_free(p->evas);
> > > - if (p->pixels) free(p->pixels);
> > > + free(p->pixels);
> > > p->pointer_object = NULL;
> > > p->hot_object = NULL;
> > > p->evas = NULL;
> > > Index: bin/e_thumb_main.c
> > > ===================================================================
> > > --- bin/e_thumb_main.c (r??vision 81358)
> > > +++ bin/e_thumb_main.c (copie de travail)
> > > @@ -220,8 +220,8 @@
> > > if (eth->objid == e->ref)
> > > {
> > > _thumblist = eina_list_remove_list(_thumblist, l);
> > > - if (eth->file) free(eth->file);
> > > - if (eth->key) free(eth->key);
> > > + free(eth->file);
> > > + free(eth->key);
> > > free(eth);
> > > break;
> > > }
> > > @@ -253,8 +253,8 @@
> > > eth = eina_list_data_get(_thumblist);
> > > _thumblist = eina_list_remove_list(_thumblist, _thumblist);
> > > _e_thumb_generate(eth);
> > > - if (eth->file) free(eth->file);
> > > - if (eth->key) free(eth->key);
> > > + free(eth->file);
> > > + free(eth->key);
> > > free(eth);
> > >
> > > if (_thumblist) _timer = ecore_timer_add(0.01, _e_cb_timer,
> > NULL);
> > > Index: bin/e_int_border_remember.c
> > > ===================================================================
> > > --- bin/e_int_border_remember.c (r??vision 81358)
> > > +++ bin/e_int_border_remember.c (copie de travail)
> > > @@ -274,12 +274,12 @@
> > > _free_data(E_Config_Dialog *cfd __UNUSED__, E_Config_Dialog_Data
> > *cfdata)
> > > {
> > > /* Free the cfdata */
> > > - if (cfdata->name) free(cfdata->name);
> > > - if (cfdata->class) free(cfdata->class);
> > > - if (cfdata->title) free(cfdata->title);
> > > - if (cfdata->role) free(cfdata->role);
> > > - if (cfdata->command) free(cfdata->command);
> > > - if (cfdata->desktop) free(cfdata->desktop);
> > > + free(cfdata->name);
> > > + free(cfdata->class);
> > > + free(cfdata->title);
> > > + free(cfdata->role);
> > > + free(cfdata->command);
> > > + free(cfdata->desktop);
> > >
> > > if (!cfdata->applied && cfdata->border->remember)
> > > {
> > > Index: bin/e_thumb.c
> > > ===================================================================
> > > --- bin/e_thumb.c (r??vision 81358)
> > > +++ bin/e_thumb.c (copie de travail)
> > > @@ -100,8 +100,7 @@
> > > if (!eth) return;
> > > eina_stringshare_replace(ð->file, file);
> > > eina_stringshare_replace(ð->key, key);
> > > - if (eth->sort_id) free(eth->sort_id);
> > > - eth->sort_id = NULL;
> > > + E_FREE(eth->sort_id);
> > > }
> > >
> > > EAPI void
> > > @@ -344,7 +343,7 @@
> > > _thumb_queue = eina_list_remove(_thumb_queue, eth);
> > > if (eth->file) eina_stringshare_del(eth->file);
> > > if (eth->key) eina_stringshare_del(eth->key);
> > > - if (eth->sort_id) free(eth->sort_id);
> > > + free(eth->sort_id);
> > > free(eth);
> > > }
> > >
> > > Index: bin/e_zone.c
> > > ===================================================================
> > > --- bin/e_zone.c (r??vision 81358)
> > > +++ bin/e_zone.c (copie de travail)
> > > @@ -660,7 +660,7 @@
> > > }
> > > }
> > > }
> > > - if (zone->desks) free(zone->desks);
> > > + free(zone->desks);
> > > zone->desks = new_desks;
> > >
> > > zone->desk_x_count = xx;
> > > Index: bin/e_fm/e_fm_ipc.c
> > > ===================================================================
> > > --- bin/e_fm/e_fm_ipc.c (r??vision 81358)
> > > +++ bin/e_fm/e_fm_ipc.c (copie de travail)
> > > @@ -1154,8 +1154,8 @@
> > > bsz = p - buf;
> > > ecore_ipc_server_send(_e_fm_ipc_server, 6 /*E_IPC_DOMAIN_FM*/, op,
> > 0, ed->id,
> > > listing, buf, bsz);
> > > - if (lnk) free(lnk);
> > > - if (rlnk) free(rlnk);
> > > + free(lnk);
> > > + free(rlnk);
> > > }
> > >
> > > static void
> > > Index: bin/e_border.c
> > > ===================================================================
> > > --- bin/e_border.c (r??vision 81358)
> > > +++ bin/e_border.c (copie de travail)
> > > @@ -5037,8 +5037,7 @@
> > > free(bd->client.netwm.icons[i].data);
> > > free(bd->client.netwm.icons);
> > > }
> > > - if (bd->client.netwm.extra_types)
> > > - free(bd->client.netwm.extra_types);
> > > + free(bd->client.netwm.extra_types);
> > > if (bd->client.border.name)
> > > eina_stringshare_del(bd->client.border.name);
> > > if (bd->bordername)
> > > @@ -6201,8 +6200,7 @@
> > > }
> > > }
> > >
> > > - if (profile)
> > > - free(profile);
> > > + free(profile);
> > >
> > > return ECORE_CALLBACK_PASS_ON;
> > > }
> > > @@ -7297,7 +7295,7 @@
> > > {
> > > char *title = ecore_x_icccm_title_get(bd->client.win);
> > > eina_stringshare_replace(&bd->client.icccm.title, title);
> > > - if (title) free(title);
> > > + free(title);
> > >
> > > if (bd->bg_object)
> > > edje_object_part_text_set(bd->bg_object, "e.text.title",
> > > @@ -7310,7 +7308,7 @@
> > > char *name;
> > > ecore_x_netwm_name_get(bd->client.win, &name);
> > > eina_stringshare_replace(&bd->client.netwm.name, name);
> > > - if (name) free(name);
> > > + free(name);
> > >
> > > if (bd->bg_object)
> > > edje_object_part_text_set(bd->bg_object, "e.text.title",
> > > @@ -7330,8 +7328,8 @@
> > > bd->client.icccm.class = eina_stringshare_add(nclass);
> > > if (bd->client.icccm.class && (!strcmp(bd->client.icccm.class,
> > "Vmplayer")))
> > > e_bindings_mapping_change_enable(EINA_FALSE);
> > > - if (nname) free(nname);
> > > - if (nclass) free(nclass);
> > > + free(nname);
> > > + free(nclass);
> > >
> > > if (!((bd->client.icccm.name == pname) &&
> > > (bd->client.icccm.class == pclass)))
> > > @@ -7412,9 +7410,7 @@
> > > }
> > > need_desk_set = EINA_TRUE;
> > > bd->client.e.state.profile.use = 1;
> > > -
> > > - if (list)
> > > - free(list);
> > > + free(list);
> > > }
> > >
> > > bd->client.e.fetch.profile = 0;
> > > @@ -7449,7 +7445,7 @@
> > > machine =
> > ecore_x_icccm_client_machine_get(bd->client.icccm.client_leader);
> > >
> > > eina_stringshare_replace(&bd->client.icccm.machine, machine);
> > > - if (machine) free(machine);
> > > + free(machine);
> > >
> > > bd->client.icccm.fetch.machine = 0;
> > > rem_change = 1;
> > > @@ -7652,7 +7648,7 @@
> > > {
> > > char *role = ecore_x_icccm_window_role_get(bd->client.win);
> > > eina_stringshare_replace(&bd->client.icccm.window_role, role);
> > > - if (role) free(role);
> > > + free(role);
> > >
> > > bd->client.icccm.fetch.window_role = 0;
> > > rem_change = 1;
> > > @@ -7661,7 +7657,7 @@
> > > {
> > > char *icon_name = ecore_x_icccm_icon_name_get(bd->client.win);
> > > eina_stringshare_replace(&bd->client.icccm.icon_name,
> > icon_name);
> > > - if (icon_name) free(icon_name);
> > > + free(icon_name);
> > >
> > > bd->client.icccm.fetch.icon_name = 0;
> > > rem_change = 1;
> > > @@ -7671,7 +7667,7 @@
> > > char *icon_name;
> > > ecore_x_netwm_icon_name_get(bd->client.win, &icon_name);
> > > eina_stringshare_replace(&bd->client.netwm.icon_name,
> > icon_name);
> > > - if (icon_name) free(icon_name);
> > > + free(icon_name);
> > >
> > > bd->client.netwm.fetch.icon_name = 0;
> > > rem_change = 1;
> >
> > >
> > ------------------------------------------------------------------------------
> > > LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
> > > Remotely access PCs and mobile devices and provide instant support
> > > Improve your efficiency, and focus on delivering more value-add services
> > > Discover what IT Professionals Know. Rescue delivers
> > > http://p.sf.net/sfu/logmein_12329d2d
> >
> > > _______________________________________________
> > > enlightenment-devel mailing list
> > > enlightenment-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> >
> >
> > --
> > Leandro Dorileo
> > ProFUSION embedded systems
> > http://profusion.mobi
> >
> >
> > ------------------------------------------------------------------------------
> > LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
> > Remotely access PCs and mobile devices and provide instant support
> > Improve your efficiency, and focus on delivering more value-add services
> > Discover what IT Professionals Know. Rescue delivers
> > http://p.sf.net/sfu/logmein_12329d2d
> > _______________________________________________
> > enlightenment-devel mailing list
> > enlightenment-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> >
--
Leandro Dorileo
ProFUSION embedded systems
http://profusion.mobi
------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
