Re: [Denemo-devel] copying the chord directives

[Richard Shann]

On Wed, 2009-07-22 at 05:10 -0500, Jeremiah Benham wrote:
> On Mon, 20 Jul 2009 17:56:27 +0100
> Richard Shann <address@hidden> wrote:
> 
> > On Sun, 2009-07-19 at 11:49 -0500, Jeremiah Benham wrote:
> > > I seem to be having issues with this. putting:
> > > 
> > > if (note->directives)
> > > list = g_list_append(list, note->directives);
> > >  in the while loop causes a segfault.
> > > 
> > > Jeremiah
> > 
> > list = g_list_append(list, note->directives) can only segfault because
> > list has been corrupted. Did you set it to NULL to start with?
> > Otherwise you are trying to append to random junk.
> > If still in difficulty email the code.
> 
> I am still having trouble with this. I am sure it is something
> extremely elementary that I am failing to understand. It copies over
> one directive only for some reason. So I have made some progress. Here
> is the code:
> 
> SCM scheme_change_chord_notes (SCM lilyname) {
call this parameter "lilynotes" (lilyname is used for other things
elsewhere), "lilynotes" conveys the idea of several notes in LilyPond
notation.
>  DenemoGUI *gui = Denemo.gui;
>  DenemoObject *curObj;
>  chord *thechord;
>  note *thenote;
>  gchar *notename;
>  gchar *chordtone;
use chordnote for this, a "tone" is a different sort of a beast, and
note is the Denemo name for a note in a chord. (The code still has a
muddled mixture of tone and note in places).
>  gint mid_c_offset;
>  gint enshift;
>  gint dclef;
>  GList *g = NULL;
>  GList *n = NULL;
>  GList *directives = NULL;
>  
>  if (scm_is_true(SCM_STRINGP(lilyname))) {
> 
>    if(!Denemo.gui || !(Denemo.gui->si)
>    || !(Denemo.gui->si->currentobject) || !(curObj =
>    Denemo.gui->si->currentobject->data) || (curObj->type!=CHORD)
>    || !(thechord = (chord *)  curObj->object) || !(thechord->notes)
>    || !(thenote = (note *) thechord->notes->data)) return
>    SCM_BOOL(FALSE); else { /* delete all chord tones */
>    while(thechord->notes){ g = g_list_append(g, thenote->directives);
you have forgotten to set thenote to (note *) thechord->notes->data, so
it remains pointing to the first note (where it was set to check that
this wasn't a rest in the boolean test above).


>           thenote->directives = NULL;
>           tonechange(gui->si, TRUE);
can you change this to delete_chordnote(gui->si)?
where
delete_chordnote(si) is a new function that calls tonechange(si, TRUE)
it is too awful as it is! It hides the fact that this function is moving
the while loop on. And if you can change the name tonechange() to
notechange while you are about it that would be good...

>         }
>         /* add changed tones */
>         dclef =  find_prevailing_clef(Denemo.gui->si);
>         notename = scm_to_locale_string(lilyname);
>         chordtone = strtok(notename, " ");
>         while (chordtone){
>           name2mid_c_offset(chordtone, &mid_c_offset, &enshift);
>           dnm_addtone (curObj, mid_c_offset, enshift, dclef);
>           chordtone = strtok( NULL, " " );
>         }
>         /* paste directives over */
>         for(n=thechord->notes;n;n=n->next) {
>           thenote = (note *) n->data;
>           directives = (DenemoDirective *) g->data;
>           
>           if (directives)
>             thenote->directives = directives;


>           
>           directives = NULL;
this statement is not needed


>           g =
>    g->next; 
move this into the for() thus:
for(n=thechord->notes;n && g;n=n->next, g=g->next)
so that all the iteration and testing is in one place.

HTH
Richard

>                                                                               
>                 
> 
>         }
>         displayhelper (gui);
>       return  SCM_BOOL(TRUE);
>    }
>  }
>  else
>    return  SCM_BOOL(FALSE);
> }
> 
> 
> Thanks,
> Jeremiah
> 
> 
> 
> 
> > Richard
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Denemo-devel mailing list
> > address@hidden
> > http://lists.gnu.org/mailman/listinfo/denemo-devel
>