Fix issue 1900 -- multiple fret-diagram-terse markups commands crash

Fix issue 1900 -- multiple fret-diagram-terse markups commands crash

This fix uses an explicit definition of the default properties for
fret-diagram-terse and fret-diagram markups, instead of copying those
from the fret-diagram-verbose markup.

No regression test will be included, since it requires a special
lilypond-book call to test.  But the Lilypond issue will have a
regression test file posted.

[Carl, 2011-11-26 01:24:03]

I've created a patch for Issue 1900.  It solves the crash, but perhaps in a
non-optimal way.

David, will you please review this, since I've changed code that you added to
scm/fret-diagrams.scm?

Thanks,

Carl

[dak, 2011-11-26 06:27:17]

Carl.D.Sorensen@gmail.com writes:

> Reviewers: ,
>
> Message:
> I've created a patch for Issue 1900.  It solves the crash, but perhaps
> in a non-optimal way.
>
> David, will you please review this, since I've changed code that you
> added to scm/fret-diagrams.scm?

You are saying that the copying of properties does not work?  I have to
review that code.  I seem to remember (but might be mistaken) that I
might have _removed_ this feature at one point of time because it
appeared unused and the code was quite awful.

-- 
David Kastrup

[dak, 2011-11-26 06:39:45]

David Kastrup <dak@gnu.org> writes:

> Carl.D.Sorensen@gmail.com writes:
>
>> Reviewers: ,
>>
>> Message:
>> I've created a patch for Issue 1900.  It solves the crash, but perhaps
>> in a non-optimal way.
>>
>> David, will you please review this, since I've changed code that you
>> added to scm/fret-diagrams.scm?
>
> You are saying that the copying of properties does not work?  I have to
> review that code.  I seem to remember (but might be mistaken) that I
> might have _removed_ this feature at one point of time because it
> appeared unused and the code was quite awful.

Does not look removed.  I might have just discussed this step in the
mailing list.  Reviewing it right now.

-- 
David Kastrup

[dak, 2011-11-26 07:32:48]

On 2011/11/26 06:39:45, dak wrote:

> Does not look removed.  I might have just discussed this step in the
> mailing list.  Reviewing it right now.

The functionality seems to be there just fine.  What makes you suspect that this
copying-of-properties-at-definition-and-documentation-time is responsible for a
crash?

Mind you, the "copying of properties" puts the whole referred function into the
properties documentation (not just a symbol or so) which is kind of icky.  It
does have the advantage that it makes it close to impossible to accidentally
create recursive references.

[Carl, 2011-11-26 15:00:13]

On 2011/11/26 07:32:48, dak wrote:
> The functionality seems to be there just fine.  What makes you suspect that
this
> copying-of-properties-at-definition-and-documentation-time is responsible for
a
> crash?

I'm just flying blind.  Mike pointed out that using properties that weren't in
an interface could lead to segfaults.  My thought was that since the whole
function was copied into the properties argument, things that weren't properties
would be set, which could trigger the segfaults that Mike had identified.

And eliminating this code eliminated the segfault.

I know you've pushed a different patch, which fixes Issue 1900, but doesn't fix
1997.  Have you tried this patch to see if it fixes 1997?

Thanks,

Carl

[dak, 2011-11-26 15:07:17]

> I know you've pushed a different patch, which fixes Issue 1900, but doesn't
fix
> 1997.  Have you tried this patch to see if it fixes 1997?

I'll do so, but frankly, if it _does_ fix 1997, I don't want it pushed.  Do you
have a reference for Mike's analysis?

It seems we should rather fix the segfault at its source than hide one way of
getting it.

[Carl, 2011-11-26 15:15:25]

On 2011/11/26 15:07:17, dak wrote:
> 
> I'll do so, but frankly, if it _does_ fix 1997, I don't want it pushed.  Do
you
> have a reference for Mike's analysis?

http://lists.gnu.org/archive/html/lilypond-devel/2011-11/msg00556.html

> 
> It seems we should rather fix the segfault at its source than hide one way of
> getting it.

I agree with you.

Thanks,

Carl

[dak, 2011-11-26 15:43:34]

On 2011/11/26 15:15:25, Carl wrote:
> On 2011/11/26 15:07:17, dak wrote:
> > 
> > I'll do so, but frankly, if it _does_ fix 1997, I don't want it pushed.  Do
> you
> > have a reference for Mike's analysis?
> 
> http://lists.gnu.org/archive/html/lilypond-devel/2011-11/msg00556.html
> 
> > 
> > It seems we should rather fix the segfault at its source than hide one way
of
> > getting it.
> 
> I agree with you.

Any way, your patch does not fix the segfault on Ubuntu 11.10 without specific
compilation options, so we can as well forget it, I guess.

Mike, any more details regarding your analysis?

If we don't have the documentation considered as an essential part of our
garbage protection scheme, it might make sense to disable _all_ documentation
for testing purposes, and then see whether we get more useful segfaults than
previously.

[Carl, 2011-11-26 16:11:01]

On 2011/11/26 15:43:34, dak wrote:
> On 2011/11/26 15:15:25, Carl wrote:

> Any way, your patch does not fix the segfault on Ubuntu 11.10 without specific
> compilation options, so we can as well forget it, I guess.
> 

OK, I'll close this issue.  We have a record of the conversation on -devel.

Thanks, David.

Carl