Closed wxtrac closed 2 years ago
wxtrac
commented
17 years ago I agree that it would be useful to restore this functionality. Of course, a patch would be the best way to address this. AFAICS the only way to fix it is to check if file length is 0 and assume it's unseekable then and expand the buffer dynamically instead of preallocating it in such case. This shouldn't be hard to do but it's not completely trivial neither.
wxtrac
commented
17 years ago I agree that it would be useful to restore this functionality. Of course, a patch would be the best way to address this.
I was thinking that perhaps you... ;)
This shouldn't be hard to do but it's not completely trivial neither. It's certainly non-trivial for me :(
Oh, all right. I've produced two patches, both of which alter the current code somewhat by putting it in one arm of an 'if' statement. This means that I've had to remove some const-ness. One of these patches uses wxMemoryBuffer for the dynamic loading, and it works. The other continues to use wxCharBuffer, and it doesn't work: for some reason that is beyond my understanding, it fails to convert the buffer to unicode.
It would also be possible to produce working code that put all of the current code in one arm of the 'if', and the 2.6.3 vintage code in the other. Apart from the obvious code duplication, that would reintroduce the chance of unicode failure that the original code had; so I've not created this.
So what would you like me to do? Put into the patch tracker the working patch? Send you the non-working patch for your care and attention? Or use the third possibility?
Thank you for your help.
David
wxtrac
commented
17 years ago The other continues to use wxCharBuffer, and it doesn't work: for some reason that is beyond my understanding, it fails to convert the buffer to unicode.
It was because I misunderstood wxCharBuffer::extend to append to the existing buffer, rather than replacing it; so I was overwriting part of the data!
It's working now, so I've submitted it as patch [1632613]. However if you'd prefer one of the other options, please let me know.
Also, for the record in case anyone else reads this in the future, some of these files return a file-length of zero, but others trigger an error and return wxInvalidOffset, which is -1.
Regards,
David
wxtrac
commented
6 years ago In cc86de14168d143543a1eb530399ffe774dbde64: Replace file reading code in wxTextFile with wxFile::ReadAll()
There is nothing special about wxTextFile justifying having code for dealing with non-seekable files in it, so put this code into wxFile itself and just call its ReadAll() method from here.
This is a better fix than 41f6f17d01562aa09bdbcc6b02241b62f1d06b75 originally applied (and which is going to be reverted next) as it doesn't break wxFile::Length() for these files and also avoids triggering an assert if the file we're trying to read was truncated by another process in the meanwhile -- which can happen and doesn't indicate a programming error and so shouldn't result in an assert.
Also add a unit test checking that this really works.
See #3802, #8354, #9965.
Issue migrated from trac ticket # 3802
priority: low
2007-01-06 23:10:21: @dghart created the issue
In wxGTK-2.6.3 and earlier I've successfully been using wxTextFile to read 'files' from the /proc virtual filesystem. In 2.7.0 and later this no longer works. The problem is because of src/common/textfile.cpp Revision 1.54, which loads the whole file into memory at once for unicode reasons. This is done by the line in wxTextFile::OnRead const size_t bufSize = (size_t)(m_file.Length() + 4 / for trailing NULs / );
This won't work for /proc, because (I suspect) it's a non-seekable filesystem. m_file.Length() always returns 0, meaning bufSize is always 4. So the Assert a few lines later is always triggered: // this shouldn't happen but don't overwrite the buffer if it does wxCHECK_MSG( bufPos + nRead <= bufSize, false, _T("read more than file length?") );
I appreciate that not many people will be attempting this sort of thing, and you might well feel that it's not worth fixing.