Re: [Denemo-devel] scm_dynwind_xxx disabled

[Richard Shann]

I've looked at (gchar*)what##_directive_get_tag (tagname) 
referenced in view.c:
  extern gchar *what##_directive_get_tag (gchar *tagname);\
  gchar *val = (gchar*)what##_directive_get_tag (tagname);\
  if(val){\
    scm_dynwind_end();\
    return scm_take_locale_stringn (val, strlen(val));;\

the implementation is in lilydirectives.c and it does indeed return a
pointer into the directive rather than a fresh copy of the tag. So the
scm_take_... is bad here because it will free the tag when the scheme
script finishes.

This will be true of all the queries for tag values, and I suspect it
will be true of many other things (e.g. asking for the object type ...)
So really, you need to justify each use of scm_take_... with a check
that the function is returning a freshly malloc'd string which the
caller must free. (I may have set you on a wild goose chase here,
perhaps there are few if any such - sorry)

Richard



On Wed, 2011-05-04 at 14:44 -0500, Jeremiah Benham wrote:
> Ok. I will look into this.
> 
> Thanks for testing,
> Jeremiah
> 
> On Wed, May 04, 2011 at 09:36:33AM +0100, Richard Shann wrote:
> > The fixes have got Denemo working again, but we have memory corruption
> > on freeing scores. I have disabled the dynwind stuff pending further
> > investigations.
> > To get a crash with the dynwind in view.c you can do this:
> > 
> >  d-New then insert a rehearsal mark and d-New again. (repeat)
> > 
> > 
> > I think we need to look at the strings to make sure they are all
> > suitable for being freed - some functions return a constant string.
> > Richard
> > 
> > 
> > On Tue, 2011-05-03 at 10:02 -0500, Jeremiah Benham wrote:
> > > I'll try to fix these issues later today.
> > > 
> > > Thanks,
> > > Jeremiah
> > > 
> > > On May 3, 2011, at 9:22 AM, Richard Shann <address@hidden>  
> > > wrote:
> > > 
> > > > a more critical example of the same thing (view.c:375):
> > > >
> > > >  SCM2LSTRING(valuename, value);\
> > > >  extern gboolean what##_directive_put_##field (gchar *tagname, gchar
> > > > *valuename);\
> > > >  scm_dynwind_end();\
> > > >  return SCM_BOOL(what##_directive_put_##field (tagname, valuename));\
> > > > }
> > > >
> > > > here valuename and tagname will already have been freed.
> > > >
> > > > Richard
> > > >
> > > >
> > > > On Tue, 2011-05-03 at 14:39 +0100, Richard Shann wrote:
> > > >> Jeremiah,
> > > >>
> > > >> I have had a read of the guile docs, I cannot claim to have a good
> > > >> understanding, but AFAICS the sort of problem we have is shown by  
> > > >> this
> > > >> example:
> > > >>
> > > >> This is a bit of the code from view.c
> > > >>
> > > >> if(name&&thepage&&post&&oth){
> > > >>   scm_dynwind_end();
> > > >>   return scm_from_locale_string(post_denemodotorg(name, thepage, oth,
> > > >> post));
> > > >> }
> > > >>
> > > >> The scm_dynwind_end() call is made before  the call to
> > > >> post_denemodotorg() which uses the values name, thepage etc as
> > > >> parameters. So I guess you need
> > > >>
> > > >> gchar *ret = post_denemodotorg(name, thepage, oth, post);
> > > >> scm_dynwind_end();
> > > >> return scm_take_locale_string(ret);
> > > >>
> > > >>
> > > >> the scm_take_locale_string() is the one to use here, as otherwise ret
> > > >> will not be freed. (Hmm, I am assuming ret *should* be freed, perhaps
> > > >> not, in which case the _from_ version is needed.)
> > > >> HTH
> > > >>
> > > >> Richard
> > > >>
> > > >>
> > > >>
> > > >> On Tue, 2011-05-03 at 09:48 +0100, Richard Shann wrote:
> > > >>> Jeremiah, I put some #defines at the top of view.c to disable the
> > > >>> dynwind stuff as the string handling seems to be very broken with  
> > > >>> it. I
> > > >>> get no custom buttons, and trying to set the score title for example
> > > >>> gives strings pointing to the wrong bits of memory.
> > > >>> Do you see this too? (I checked with Nils clean build which shows  
> > > >>> the
> > > >>> problem).
> > > >>> Richard
> > > >>>
> > > >>>
> > > >>>
> > > >>> _______________________________________________
> > > >>> Denemo-devel mailing list
> > > >>> address@hidden
> > > >>> https://lists.gnu.org/mailman/listinfo/denemo-devel
> > > >>
> > > >>
> > > >> _______________________________________________
> > > >> Denemo-devel mailing list
> > > >> address@hidden
> > > >> https://lists.gnu.org/mailman/listinfo/denemo-devel
> > > >
> > > >
> > > > _______________________________________________
> > > > Denemo-devel mailing list
> > > > address@hidden
> > > > https://lists.gnu.org/mailman/listinfo/denemo-devel
> >