No, that's not following my logic. We need to make the common case super awesome, and the less common case awesome, and I think the common case is not deleting NULL objects, but just managing them properly. Therefore lets make the common case (NULL deletion is unwanted) super awesome and the less common case (NULL deletion is wanted) as just awesome, and print those damn warnings. :)
-- Tom. On 03/10/12 14:26, Michael Blumenkrantz wrote: > by your logic, there's no reason why free(NULL) is ever needed either, > but it still exists and is widely used. it's a convenience, and it's one > which makes a lot of sense. if a developer is so incompetent that s/he > can't even manage objects well enough to pass one to a free function, > then imo this person should not be using our libraries at all, and I'm > fine with leaving this commit in place explicitly to keep that person > away from our helpdesk. > > On Wed, Oct 3, 2012 at 1:22 PM, Tom Hacohen <tom.haco...@samsung.com > <mailto:tom.haco...@samsung.com>> wrote: > > I know what evas_objec_free() does, and it is not like free, even if > the object is actually freed, because there's a graphical > representation to the object, and it's an object on screen (in most > cases). So just having a pointer that may be NULL and deleting it > just in case is way less common than just freeing pointers. > > Since there's a graphical representation to objects, i.e they are > not "internal magic thingies", one should be meticulous when > managing them, and not just have a pointer that may be null and may > delete an object. I can think of very little cases that such a > behaviour will be needed, and in other cases it'll just be hiding > bugs of developers that forgot to manage their objects right and > expected something to happen that doesn't. > > -- > Tom. > > On 03/10/12 14:17, Michael Blumenkrantz wrote: > > I thought that my comment would be sufficient to convey the > implication > that this is a free case. I was wrong, so I'll be more explicit: > > this is a case where an object is freed in nearly every case. > the only > time this is not true is if you've previously called > evas_object_ref(), > in which case evas_object_unref() will be called as some point. > the only > reason I didn't add it here too is because it's so much less common. > > passing NULL here is not a bug. there is no unwanted behavior > that can > possibly result here from code which does this. there are no > downsides > to it. you're just being pedantic without reason. > > On Wed, Oct 3, 2012 at 1:11 PM, Tom Hacohen > <tom.haco...@samsung.com <mailto:tom.haco...@samsung.com> > <mailto:tom.haco...@samsung.com > <mailto:tom.haco...@samsung.com>>> wrote: > > I said it's *not* like the free case... > > On 03/10/12 14:05, Michael Blumenkrantz wrote: > > passing NULL to a free function is not an error case. if it > were, then > free(NULL) would crash. > > On Wed, Oct 3, 2012 at 12:59 PM, Tom Hacohen > <tom.haco...@samsung.com > <mailto:tom.haco...@samsung.com> <mailto:tom.haco...@samsung.com > <mailto:tom.haco...@samsung.com>>>wrote: > > I agree with Gustavo. This is as obviously an error > case, > and errors > should be printed as such. It's not a convenience like > making free check > for NULL, as deleting objects has more meaning than > just > freeing memory. > > I think one of the best improvements I've ever did > to Evas > was turning > magic/null bitching on for everything, it's just makes > finding bugs > easier, and doesn't let you sweep your errors under > the rug. > > -- > Tom. > > On 03/10/12 13:53, Gustavo Sverzut Barbieri wrote: > > Bad commit. > > At least if EVAS_MAGIC_SOMETHING variable is set we > should bitch so we > > can > > debug these errors > > On Wednesday, October 3, 2012, Enlightenment > SVN wrote: > > Log: > probably the best commit I'll ever make: > evas_object_del() now takes > > NULL > > parameter without bitching > > > Author: discomfitor > Date: 2012-10-02 23:59:20 -0700 > (Tue, 02 Oct > 2012) > New Revision: 77344 > Trac: > http://trac.enlightenment.org/e/changeset/77344 > > Modified: > trunk/evas/ChangeLog trunk/evas/NEWS > trunk/evas/src/lib/canvas/evas_object_main.c > > Modified: trunk/evas/ChangeLog > > =================================================================== > --- trunk/evas/ChangeLog 2012-10-03 > 06:41:32 > UTC (rev 77343) > +++ trunk/evas/ChangeLog 2012-10-03 > 06:59:20 > UTC (rev 77344) > @@ -1067,3 +1067,7 @@ > > * Add the object display mode > hint. this > can be used to check > object > mode like compress or expand or etc > + > +2012-10-03 Mike Blumenkrantz > + > + * evas_object_del() now accepts > NULL more > peacefully > > Modified: trunk/evas/NEWS > > =================================================================== > --- trunk/evas/NEWS 2012-10-03 06:41:32 > UTC (rev > 77343) > +++ trunk/evas/NEWS 2012-10-03 06:59:20 > UTC (rev > 77344) > @@ -14,7 +14,8 @@ > > * Function to rotate an evas map with a > quaternion: > evas_map_util_quat_rotate(). > * EVAS_GL_NO_BLACKLIST env var to > turn off > blacklisted drivers in > > gl > > - * Evas gl enigne can do partial swaps now. > + * Evas gl engine can do partial swaps now. > + * evas_object_del() now takes NULL > parameters > > Fixes: > > > Modified: > trunk/evas/src/lib/canvas/evas_object_main.c > > =================================================================== > --- > trunk/evas/src/lib/canvas/evas_object_main.c > 2012-10-03 > 06:41:32 UTC (rev 77343) > +++ > trunk/evas/src/lib/canvas/evas_object_main.c > 2012-10-03 > 06:59:20 UTC (rev 77344) > @@ -430,6 +430,7 @@ > EAPI void > evas_object_del(Evas_Object *obj) > { > + if (!obj) return; > MAGIC_CHECK(obj, Evas_Object, > MAGIC_OBJ); > return; > MAGIC_CHECK_END(); > > > > > > > ------------------------------------------------------------------------------ > > Don't let slow site performance ruin your > business. > Deploy New Relic APM > Deploy New Relic app performance management > and know > exactly > what is happening inside your Ruby, Python, > PHP, > Java, and .NET app > Try New Relic at no cost today and get our > sweet > Data Nerd shirt too! > http://p.sf.net/sfu/newrelic-dev2dev > _______________________________________________ > enlightenment-svn mailing list > enlightenment-...@lists.sourceforge.net > <mailto:enlightenment-...@lists.sourceforge.net> > > <mailto:enlightenment-...@lists.sourceforge.net > <mailto:enlightenment-...@lists.sourceforge.net>> > <javascript:;> > https://lists.sourceforge.net/lists/listinfo/enlightenment-svn > > > > > > > > > ------------------------------------------------------------------------------ > Don't let slow site performance ruin your business. > Deploy > New Relic APM > Deploy New Relic app performance management and > know exactly > what is happening inside your Ruby, Python, PHP, > Java, and > .NET app > Try New Relic at no cost today and get our sweet > Data Nerd > shirt too! > http://p.sf.net/sfu/newrelic-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > <mailto:enlightenment-devel@lists.sourceforge.net> > <mailto:enlightenment-devel@lists.sourceforge.net > <mailto:enlightenment-devel@lists.sourceforge.net>> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > ------------------------------------------------------------------------------ > Don't let slow site performance ruin your business. > Deploy New > Relic APM > Deploy New Relic app performance management and know > exactly > what is happening inside your Ruby, Python, PHP, Java, > and .NET app > Try New Relic at no cost today and get our sweet Data > Nerd shirt > too! > http://p.sf.net/sfu/newrelic-dev2dev > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > <mailto:enlightenment-devel@lists.sourceforge.net> > <mailto:enlightenment-devel@lists.sourceforge.net > <mailto:enlightenment-devel@lists.sourceforge.net>> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > ------------------------------------------------------------------------------ Don't let slow site performance ruin your business. Deploy New Relic APM Deploy New Relic app performance management and know exactly what is happening inside your Ruby, Python, PHP, Java, and .NET app Try New Relic at no cost today and get our sweet Data Nerd shirt too! http://p.sf.net/sfu/newrelic-dev2dev _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
