[sword-devel] Patch for Sword crash with BibleTime on Windows

[Troy A. Griffitts]

Gary,

Thanks again for tracking this down and reporting the problem.  I just
looked at your comment and it does look like this would be a good fix.

The oddity is that it looks the bug stems from the Personal module.  The
RawFiles::createModule method does the right thing and create the
increment number holder file, incfile, and initialized it to 0.  Anyone
creating a new personal commentary with this method should be fine.

The Personal module as shipped from the CrossWire repository does not
include this file!  Hence the problem of not finding the file.

So, I suppose we could chalk this up to a module error just as well as
code which could better handle problem conditions like a missing module
file.

Awesome find!  I wonder how long the Personal module has shipped this
way!  I wonder why BibleCS hasn't had any trouble, as I'm pretty sure
the only way to use personal commentaries is to download the module.
Maybe Borland's entry into the Win32 read call handles bad file pointers
better?

In anycase, I like your solution the best, in that we check that we
successfully oped a file before we attempt to write to it.  Thank you again,

Troy



On 02/12/2011 04:03 AM, Gary Holmlund wrote:
> Troy,
> 
> Looking at getNextFilename more, it seems that it should skip reading
> the file if it did not open and go on with writing the incfile a few
> lines later.
> 
> Gary
> 
> 
> On 2/11/2011 7:39 PM, Gary Holmlund wrote:
>> Troy,
>>
>> In Rawfiles::getNextFilename() it is opening this file to read it.
>> "C:\ProgramData\Application
>> Data\Sword/modules/comments/rawfiles/personal/incfile"
>>
>> The problem is that this file does not exist. I am not sure when this
>> file should have been created.
>>
>> Gary
>>
>>
>> On 2/11/2011 5:49 PM, Troy A. Griffitts wrote:
>>> Hey Gary,
>>>
>>> Thanks for the report.  The problem with your patch is that it doesn't
>>> actually fix the problem.  read should fail if you try to read from an
>>> invalid file descriptor.  The problem is that it sounds like the
>>> RawFiles driver has a bug which is reading from an invalid file
>>> descriptor.
>>>
>>> Does this make sense?  I appreciate the report.  We should track this
>>> down.  And I appreciate that this fixes your specific problem, but it
>>> changes the behavior of the SWORD read method to be different from the
>>> libc read method and I don't think we should do this.
>>>
>>> If you can track down the problem in RawFiles near line 194, that would
>>> be excellent!
>>>
>>> Thanks again for the report,
>>>
>>> Troy
>>>
>>>
>>>
>>> On 02/12/2011 01:15 AM, Gary Holmlund wrote:
>>>> Hi,
>>>>
>>>> I just rediscovered a sword bug that I told you about over a year ago.
>>>> For Bibletime it crashes the program when you try to use the Personal
>>>> Commentary in write mode on Windows.
>>>>
>>>> I upgraded to Sword 1.62 recently and this caused the bug to occur
>>>> again. I had been using a patched version of 1.6.0 for our Windows
>>>> build. Please incorporate the patch this time.
>>>>
>>>> Thanks,
>>>>
>>>> Gary Holmlund
>>>>
>>>> --------------------------------------------------------------------------------
>>>>
>>>> *Gary Holmlund* gary.holmlund at gmail.com
>>>> <mailto:sword-devel%40crosswire.org?Subject=Re:%20Re%3A%20%5Bsword-devel%5D%20Patch%20for%20Sword%20crash%20with%20BibleTime%20on%20Windows&In-Reply-To=%3C4AF0ECE1.6090906%40gmail.com%3E>
>>>>
>>>> /Tue Nov 3 19:54:25 MST 2009/
>>>>
>>>>      * Previous message: [sword-devel] Python client
>>>> <http://www.crosswire.org/pipermail/sword-devel/2009-November/033042.html>
>>>>
>>>>      * Next message: [sword-devel] Python client
>>>> <http://www.crosswire.org/pipermail/sword-devel/2009-November/033044.html>
>>>>
>>>>      * *Messages sorted by:* [ date ]
>>>> <http://www.crosswire.org/pipermail/sword-devel/2009-November/date.html#33043>
>>>>
>>>>        [ thread ]
>>>> <http://www.crosswire.org/pipermail/sword-devel/2009-November/thread.html#33043>
>>>>
>>>>        [ subject ]
>>>> <http://www.crosswire.org/pipermail/sword-devel/2009-November/subject.html#33043>
>>>>
>>>>        [ author ]
>>>> <http://www.crosswire.org/pipermail/sword-devel/2009-November/author.html#33043>
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>>
>>>> Hi,
>>>>
>>>> I am working on BibleTime for Windows and we found and fixed a crash in
>>>> the sword library. I have attached a patch for the HEAD of sword
>>>> svn. We
>>>> are using the 1.60 version of sword.
>>>>
>>>> The crash occurs when trying to save to a personal commentary for the
>>>> first time. Sword is looking for the "incfile" of the personal
>>>> commentary. The file does not exist yet.. Here is the call stack and
>>>> function at the crash point.
>>>>
>>>> libsword.dll!sword::FileDesc::read(void * buf=0x013eaec0, long count=4)
>>>> Line 139
>>>> libsword.dll!sword::RawFiles::getNextFilename()  Line 194
>>>> libsword.dll!sword::RawFiles::setEntry(const char * inbuf=0x0253d050,
>>>> long len=3)  Line 130
>>>> bibletime.exe!CSwordModuleInfo::write(CSwordKey * key=0x02e6cd00, const
>>>> QString&  newText={...})  Line 705
>>>>
>>>> long FileDesc::read(void *buf, long count) {
>>>>     return ::read(getFd(), buf, count);       // crash here
>>>> }
>>>>
>>>> Since the file does not exist, getFd() returns a  fd of -1. The read
>>>> promptly crashes with the negative fd. Tracing the same problem in
>>>> linux
>>>> shows the same -1 fd, but the read does not crash there. The crash
>>>> seems
>>>> specific to the Visual Studio 2008 runtime libraries, but reading
>>>> with a
>>>> negative fd value is clearly wrong.
>>>>
>>>> The fix is simple. Test for the negative fd and return 0 from
>>>> fileDesc::read if fd is negative. Returning 0 is what is happening
>>>> on linux.
>>>>
>>>> Gary Holmlund
>>
> 
> 
> _______________________________________________
> 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