[sword-devel] breakage in verse management at -r2785?

[Greg Hellings]

Test code: https://gist.github.com/greg-hellings/5218094

Output with your current patch, Troy, is $ g++ `pkg-config --cflags sword`
`pkg-config --libs sword` test.cpp -o test && ./test
intro:  0
bk (1): 1
ch (1): 1
vs (1): 1
-----------------
intro:  0
bk (1): 0
ch (1): 0
vs (1): 0
-----------------
intro:  1
bk (0): 0
ch (0): 0
vs (0): 0
-----------------

The value in parenthsis represents what I thought the value "should" be. If
I comment out line 19, I get this result $ g++ `pkg-config --cflags sword`
`pkg-config --libs sword` test.cpp -o test && ./test
intro:  0
bk (1): 1
ch (1): 1
vs (1): 1
-----------------
intro:  0
bk (1): 1
ch (1): 1
vs (1): 1
-----------------
intro:  1
bk (0): 0
ch (0): 0
vs (0): 0
-----------------

which matches the expected results. So Xiphos is getting around calling
setIntros(1) by instead disabling auto-normalizing. It's odd to me that you
can set a key to a value in the intros while setIntros is false. That
doesn't seem like normalizing to me, that seems more like bounds checking,
but that's not necessarily a bug in the API, possibly it's a bug in my
understanding of intended behavior.

Troy's fix along with shuffling the location of setTestament (a change I
made earlier to Xiphos' Subversion) fixes the known bugs I've seen in
Xiphos regarding chapters not rendering and Genesis 1:1 causing a SegFault.

--Greg



On Thu, Mar 21, 2013 at 6:51 PM, Greg Hellings <greg.hellings at gmail.com>wrote:

>
>
>
> On Thu, Mar 21, 2013 at 6:26 PM, Troy A. Griffitts <scribe at crosswire.org>wrote:
>
>> Thanks Karl,
>>
>> Yes, each snippet was helpful. Nic's was a quick test which caused the
>> bug and was easy to use for testing. Greg's snippet wasn't as helpful as
>> all his comments and stack traces leading up to his patch. He is preventing
>> book from getting to 0 which does alleviate the problem but also stops a
>> book from becoming an intro. Hope that explains.
>>
>
> If the conditional in my code is changed to
>
> if (book > (intros?0:1) ) {
>
> Then my code corrects the problem that -- was moving a VerseKey from
> Genesis 1:1 to [Testament 1 Intro] when intros is false. While not a
> SegFault producing bug, this is still a bug. Similar checks should be made
> through the same run of code to ensure moving to verse 0 and chapter 0 are
> not possible when intros is false. Both your fix and an adapted form of
> mine should be introduced. Additionally, the issue with OSISFootnotes
> should probably be fixed.
>
> I will write up a simple test case for that, if it helps, once I get back
> to my development machine.
>
> --Greg
>
>
>>
>> Troy
>>
>> Karl Kleinpaste <karl at kleinpaste.org> wrote:
>>>
>>> You didn't also need this snippet from Greg a couple days ago?
>>>
>>> --- src/keys/versekey.cpp (revision 2792)
>>>
>>> +++ src/keys/versekey.cpp (working copy)
>>> @@ -1347,7 +1347,9 @@
>>>    }
>>>    if (verse < (intros?0:1)) {
>>>     if (--chapter < (intros?0:1)) {
>>> -     --book;
>>> +     if (book > 1) {
>>> +      --book;
>>> +     }
>>>
>>>      chapter += (getChapterMax() + (intros?1:0));
>>>     }
>>>     verse += (getVerseMax() + (intros?1:0));
>>>
>>> ------------------------------
>>>
>>> sword-devel mailing list: sword-devel at crosswire.org
>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>> Instructions to unsubscribe/change your settings at above page
>>>
>>>
>> --
>> Sent from my Android phone with K-9 Mail. Please excuse my brevity.
>>
>> _______________________________________________
>>
>> sword-devel mailing list: sword-devel at crosswire.org
>> http://www.crosswire.org/mailman/listinfo/sword-devel
>> Instructions to unsubscribe/change your settings at above page
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/sword-devel/attachments/20130321/c34ef8d1/attachment-0001.html>