Re: [Denemo-devel] scm_dynwinds etc. [was:] Midi shortcuts

[Richard Shann]

On Fri, 2011-05-13 at 06:14 +0200, Jeremiah Benham wrote:
> Does this look like I am understanding this stuff:
> 
> #define GET_TAG_FN_DEF(what)\
>  static SCM scheme_##what##_directive_get_tag(SCM tag) {\
>   scm_dynwind_begin(0);\
>   gchar *tagname;\
>   if(!scm_is_string(tag))\
>      tagname = NULL;\
>   else { \
>     tagname = scm_to_locale_string(tag);\
>     scm_dynwind_unwind_handler (g_free, tagname, SCM_F_WIND_EXPLICITLY);\
>   } \
>   extern gchar *what##_directive_get_tag (gchar *tagname);\
>   gchar *val = (gchar*)what##_directive_get_tag (tagname);\
>   if(val){\
>     scm_dynwind_end();\
>     SCM ret = scm_from_locale_stringn (val, strlen(val));\
>     return ret;\
>   }\
>   scm_dynwind_end();\
>   return SCM_BOOL(FALSE);\
> }
> 
> I am not sure if val
that is a string pointing into to the directive and must not be freed
except when the directive is being deleted.
>  or tagname need to be freed using g_free.
tagname has been created by the scheme library and we are told to use
free() for it when we are finished.
>  In regards to tagname the docs say:
> 
> C Function: char * scm_to_locale_string (SCM str)
> returns a C string with the same contents as str in the character encoding of 
> the current locale. The C string must be freed with free eventually, maybe by 
> using scm_dynwind_free, See Dynamic Wind.
> 
> The thing I find is confusing is that it says maybe.
yes, that single word "maybe" was what started me on this wild goose
chase. "maybe" means if you call scheme so that it may never return to
you, and so you wouldn't get a chance to free tagname. We don't do that.
>  Should I have a g_free(tagname) also? When it comes to val I am not sure if 
> I am able to free this without causing a problem. What can we do to clear 
> this up so that it is odvious if the gchar * is to be freed or not. Should we 
> have these functions only return const gchar * 
yes, I think so, although I believe that the C language unfortunately
prevents the compiler from exploiting the const declaration to optimize
unlike some other languages. So this is more decorative than anything.
> or something like that?
> 
> Jeremiah
> 

I don't think we need scm_dynwind etc in here at all, since we don't
call scheme in this function. So I think we need this

#define GET_TAG_FN_DEF(what)\
 static SCM scheme_##what##_directive_get_tag(SCM tag) {\
  gchar *tagname;\
  if(!scm_is_string(tag))\
     tagname = NULL;\
  else { \
    tagname = scm_to_locale_string(tag);\
  } \
  extern gchar *what##_directive_get_tag (gchar *tagname);\
  gchar *val = (gchar*)what##_directive_get_tag (tagname);\
  free(tagname);\
  if(val){\
    SCM ret = scm_from_locale_stringn (val, strlen(val));\
    return ret;\
  }\
  return SCM_BOOL(FALSE);\
}

I haven't got a toolchain installed yet, so I can't even compile this,
but that is my idea of what this should look like.
The only case that is different is where the string val is a newly
allocated one which has to be freed by the caller. I don't think there
are many of these (I suspect that when we return a Clef type for
instance it is a constant) but there may be some.

HTH

Richard


> 
> 
> 
> On Wed 11/05/11 12:56 PM , "R. Mattes"  wrote::
> 
> On Wed, 11 May 2011 17:30:24 +0100, Richard Shann wrote
> 
> > > >  More importantly the scm_take_from... need to revert to scm_from... 
> > > > in all cases. In the cases where the string being passed is freshly 
> > > > allocated (ie g_malloc'd) then call g_free() on it having created 
> > > > the scm_from... SCM structure.
> > > 
> > > And dont' forget to register it with the dynamic context! ;-)
> > 
> > it = the return SCM??? see above - do we need to do something to say
> > that the SCM returned by our C primitive procedure (gsubr...) should 
> > be freed?
> 
> No, SCM values are taken care of by guile's garbage colector.
> You only need to take care of heap-allocated resources not-SCM.
> BTW, if you allocate something with g_malloc the corresponding context
> registering should look like:
> 
>  a_blob *my_thomething = g_malloc(sizeof(a_blob));
> 
> scm_dynwind_unwind_handler (g_free, a_blob, SCM_F_WIND_EXPLICITLY);
> 
>  
> > >  
> > > > We can't exploit scm_take_from... (as 
> > > > I previously suggested) because it calls free() and glib  is using 
> > > > its own memory allocator (so g_free() doesn't call free()). (So, 
> > > > another wild goose chase started by me I'm afraid :(
> > > 
> > > Ah, good to know. Anyway, looking at scm_take_from... in Guile 2 there 
> > > will
> > > be no benefit performance-wise anyway. 
> > >  
> > > > It is only not crashing at the moment, because it seems scheme is not
> > > > collecting its garbage...
> > > 
> > > Have you tested with manual gc?
> > No, I'm afraid I haven't done any coding for a couple of days, but I
> > have realized as a result of all this that my treatment of the Undo
> > stack in the case of a script throwing an error is unnecessarily
> > elaborate, and I was just going to remove that and to deactivate the
> > scm_take_ calls because someone could potentially download the source
> > and compile on a system where guile would garbage collect with
> > disastrous results.
> 
> I agree, scm_take_... sounds like more hazzle than it's worth.
> 
>  RalfD
> 
> > >  Cheers, RalfD
> 
> 
> --
> R. Mattes -
> Hochschule fuer Musik Freiburg
> address@hidden
> 
> 
> 
>