From: David F. <fr...@tu...> - 2009-08-28 15:24:33
|
Hi all, The Lib/perl/perlrun.swg file needs a minor patch: Index: perlrun.swg =================================================================== --- perlrun.swg (revision 11670) +++ perlrun.swg (working copy) @@ -293,7 +293,6 @@ int newmemory = 0; *ptr = SWIG_TypeCast(tc,voidptr,&newmemory); if (newmemory == SWIG_CAST_NEW_MEMORY) { - assert(own); if (own) *own = *own | SWIG_CAST_NEW_MEMORY; } HTH -- David Fletcher Tuscany Design Automation, Inc. dav...@tu... 3030 S. College Ave. 720-207-9278 Ft. Collins, CO 80525 USA |
From: Olly B. <ol...@su...> - 2009-08-29 06:57:13
|
On 2009-08-28, David Fletcher <fr...@tu...> wrote: > --- perlrun.swg (revision 11670) > +++ perlrun.swg (working copy) > @@ -293,7 +293,6 @@ > int newmemory = 0; > *ptr = SWIG_TypeCast(tc,voidptr,&newmemory); > if (newmemory == SWIG_CAST_NEW_MEMORY) { > - assert(own); > if (own) > *own = *own | SWIG_CAST_NEW_MEMORY; > } Thanks, applied in r11671. Presumably it does actually work if the assertion is removed? Cheers, Olly |
From: William S F. <ws...@fu...> - 2009-08-29 10:03:44
|
Olly Betts wrote: > On 2009-08-28, David Fletcher <fr...@tu...> wrote: >> --- perlrun.swg (revision 11670) >> +++ perlrun.swg (working copy) >> @@ -293,7 +293,6 @@ >> int newmemory = 0; >> *ptr = SWIG_TypeCast(tc,voidptr,&newmemory); >> if (newmemory == SWIG_CAST_NEW_MEMORY) { >> - assert(own); >> if (own) >> *own = *own | SWIG_CAST_NEW_MEMORY; >> } > > Thanks, applied in r11671. > > Presumably it does actually work if the assertion is removed? > Actually, it is important to leave in that assert. If the assert fails, then you are guaranteed a memory leak. It indicates that the new memory handling is not taken care of in the appropriate typemap, so this assert is an essential debugging aid. William |
From: Olly B. <ol...@su...> - 2009-09-02 11:12:24
|
On Sat, Aug 29, 2009 at 11:03:30AM +0100, William S Fulton wrote: > Actually, it is important to leave in that assert. If the assert fails, > then you are guaranteed a memory leak. It indicates that the new memory > handling is not taken care of in the appropriate typemap, so this assert > is an essential debugging aid. Hmm, this seems to be rather abusing assert() and is problematic if the user is trying to wrap inline code which uses assert() since they can't disable this assert() independently of those in their own code... Perhaps it would be better to have a C macro to enable this leaking code, and otherwise call abort() or some other suitable action if there is one? Is this leak fixed in talby's branch? Cheers, Olly |
From: William S F. <ws...@fu...> - 2009-09-02 20:58:07
|
Olly Betts wrote: > On Sat, Aug 29, 2009 at 11:03:30AM +0100, William S Fulton wrote: >> Actually, it is important to leave in that assert. If the assert fails, >> then you are guaranteed a memory leak. It indicates that the new memory >> handling is not taken care of in the appropriate typemap, so this assert >> is an essential debugging aid. > > Hmm, this seems to be rather abusing assert() and is problematic if the > user is trying to wrap inline code which uses assert() since they can't > disable this assert() independently of those in their own code... > I'm not sure I follow what inline has got to do with this. By fixing the appropriate typemaps, the assert won't trigger. I don't see what is wrong with this approach, it's a pretty standard defensive programming technique. I suppose we could display a memory leak warning for release builds in addition to the assert(). > Perhaps it would be better to have a C macro to enable this leaking > code, and otherwise call abort() or some other suitable action if there > is one? > > Is this leak fixed in talby's branch? This assert is for the smart pointer casting support which David Fletcher added, and there havn't been any merges of this into talby's branch. The assert is really a defensive programming technique just for the developers of the smart pointer typemaps. William |
From: Olly B. <ol...@su...> - 2009-09-03 11:13:27
|
On Wed, Sep 02, 2009 at 09:57:51PM +0100, William S Fulton wrote: > This assert is for the smart pointer casting support which David > Fletcher added, and there havn't been any merges of this into talby's > branch. The assert is really a defensive programming technique just for > the developers of the smart pointer typemaps. Ah, I'd misunderstood the context - I thought you were saying that the assert() was marking a typemap which was known to be implemented incorrectly and leak, and the idea was that users could turn off assertions if they were happy to accept the leak. But it sounds like it actually is catching a situation which shouldn't be possible, so if it fails, there's a bug in SWIG (which of course is a totally fine use of an assertion). I've reverted this change. I think it would be useful to stick a comment by the assertion to explain what it means if it fails, since David didn't see it and neither did I, but I'm still not sure exactly what it actually means. I can add something vague, but if you can sum it up briefly, that'd be better. Cheers, Olly |