[C++, MSVC] bug istream_iterator na fstream

Witam,
Nie bardzo mogę sobie poradzić z komunikatem jaki dostaję przy uruchomieniu programu:
istream_iterator is not dereferencable, sama kompilacja przebiega prawidłowo bez najmniejszego nawet ostrzeżenia. Ten sam kod kompilowany przez gcc uruchamia sie i działa bardzo szybko nawet z dużymi plikami.
Może ma ktoś jakieś sugestie? Albo może pomysł jak zrobić to samo tylko innym sposobem?

#include <fstream>
#include <iterator>
#include <algorithm>

using namespace std;

int main() {

    typedef istream_iterator<unsigned char> input_iter_t;

    const off_t SIZE = 4;
    char before[SIZE] = { 0x12, 0x34, 0x56, 0x78 };
    char after[SIZE] = { 0x69, 0x74, 0x65, 0x72 };

    fstream filestream("numbers.exe", ios::binary | ios::in | ios::out);

    if (search(input_iter_t(filestream), input_iter_t(), before, before + SIZE) != input_iter_t()) {
        filestream.seekp(-SIZE, ios::cur);
        filestream.write(after, SIZE);
    }
}

Dzieki,
J.

wewnatrz ciala search:
\VC\include\algorithm, linie 204-214:

	for (; _Count2 <= _Count1; ++_First1, --_Count1)
		{	// room for match, try it
		_FwdIt1 _Mid1 = _First1;
		for (_FwdIt2 _Mid2 = _First2; ; ++_Mid1, ++_Mid2)
			if (_Mid2 == _Last2)
				return (_First1);
			else if (!(*_Mid1 == *_Mid2))  // <---------------------------
				break;
		}
	return (_Last1);
	}

pierwszy obieg petli, przejscie przez oznaczona linie, wynik false, nawrot petli, wykonuje sie ++_First1 i w tym momencie iterator odrywa sie od strumienia.
czemu? jeszcze nie wiem. wykonanie reczne analogicznego:

	input_iter_t tmp = start;
	unsigned char test = *tmp;
	++tmp;
	unsigned char test2 = *tmp; 

nie powoduje bledu, natomiast skopiowanie petli z algorithm:search -- powoduje w 100% ten sam blad.

to tyle po 15minutach..

ok, juz mam: ciezko mi powiedziec, czy jest to blad w implementacji std::ifstream, czy tez raczej szczegol implementacyjny, jednak mi osobiscie to sie nie zgadza i sadze ze jednak blad. czy rzeczywiscie - trzebaby spojrzec na specyfikacje..

w kazdym badz razie, problem brzmi: _Distance. O to caly snip z algorithm, z usunietymi smieciami:

//_FwdIt1 = std::istream_iterator<unsigned char,char,std::char_traits<char>,int>
//_FwdIt2 = char*
//_Diff1 = int
//_Diff2 = int
template<class _FwdIt1, class _FwdIt2, class _Diff1, class _Diff2>
inline _FwdIt1 _Search(_FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _FwdIt2 _Last2, _Diff1 *, _Diff2 *)
{
    // find first [_First2, _Last2) match
    _Diff1 _Count1 = 0;
    _Distance(_First1, _Last1, _Count1);   // <--------------- przyczyna

    _Diff2 _Count2 = 0;
    _Distance(_First2, _Last2, _Count2);

    for (; _Count2 <= _Count1; ++_First1, --_Count1)
    {
        // room for match, try it
        _FwdIt1 _Mid1 = _First1;
        for (_FwdIt2 _Mid2 = _First2; ; ++_Mid1, ++_Mid2)
            if (_Mid2 == _Last2)
                return (_First1);
            else if (!(*_Mid1 == *_Mid2))   // <--------------- crash
                break;
    }
    return (_Last1);
}

test:

    _Distance(start, end, n);
    input_iter_t tmp = start;
    unsigned char test = *tmp;
    ++tmp;
    unsigned char test2 = *tmp; // crash

implementacja Distance jest hem dosc łopatologiczna:

for (; _First != _Last; ++_First) ++_Off;

tak wiec, Distance powoduje przewiniecie KOPII iteratora az do konca. reczny test pokazuje, ze crash wystepuje tylko w takim przypadku. testowy ifstream ma 87576 bajtow:

	input_iter_t tmp = start;
	for(int i = 0; i< 87576; ++tmp, ++i);
	unsigned char test = *tmp; // crash. strumien ZAMKNIETY

	input_iter_t tmp = start;
	for(int i = 0; i< 87576-1; ++tmp, ++i);
	unsigned char test = *tmp;  // ok
	++tmp;
	unsigned char test2 = *tmp; //crash
	unsigned char test3 = *start; // crash

a wszystko robija sie o iteratory:

    input_iter_t tmp;
	
    tmp = start;     unsigned char t0a = *tmp;    // 'M'    plik, bajt numer 0
    advance(tmp, 1); unsigned char t1a = *tmp;    // 'Z'    plik, bajt numer 1
    advance(tmp, 1); unsigned char t2a = *tmp;    // 0x90    plik, bajt numer 2

    tmp = start;     unsigned char t0b = *tmp;    // 'M'    plik, bajt numer 0
    advance(tmp, 1); unsigned char t1b = *tmp;    // 0x00    plik, bajt numer 3
    advance(tmp, 1); unsigned char t2b = *tmp;    // 0x03    plik, bajt numer 4

mam nadzieje, ze to wyjasnia przyczyne crasha pod MSVC.

chodzenie iteratorami nie powinno jej zmieniac zrodlowego obiektu w sposob znaczacy, a same iteratory powinny byc kopiowalne i pamietac swoj stan.
one zmieniaja podlegly strumien, i maja zbyt wąski stan własny - pamiętaja odczytana wartość, ale nie pamiętają pozycji!
++/-- wydaje sie generować nowy iterator na podstawie aktualnej pozycji strumienia, a nie na podstawie pozycji oryginalnego iteratora.
IMHO, albo BUG implementacji, albo BUG specyfikacji biblioteki std z microsoftu.

heh - to pewnie wynika z zalozenia, ze to jest INPUT iterator, jedynie inkrementowalny, niecofalny.
niemniej, sadze, ze powinien byc bezpiecznie kopiowalny, skoro ma copyctor i kropka.

testowane na VS2008ProSp

//edit:
patrzac na kod istream_iterator --- jest WYRAZNIE bezstanowy, abstrahujac od cache'owana wartosci.
on pod spodem uzywa operatora >> aby wyciagnac ze strumienia wartosc.
to chyba wszystko tlumaczy w 100%

sorry-batorry, trzeba machnac swoj iterator plus ew. wrapper wybierajacy implementacje miedzy g++ a msvc..

//edit:
najlepsze 2 poprawki łatające owo zachowanie istream_iterator'a jakie udalo mi sie zrobic, spowalniaja go 7.77x (ogolna) i 4.67x (tylko dla unsigned char), wiec nie publikuje.. zajalbym sie wiec raczej napisaniem tego lekko na okolo, lub skopiowaniem kodu std::search z g++, o ile nie uzywa ono Distance

Dzięki za bardzo szczrgółowe wyjaśnienie ale czy masz może jakieś bardziej praktyczne rozwiazanie?
Probowałem jeszcze zrobić to tak jak poniżej ale dziala okropnie wolno.

#include <iostream>
#include <cstdlib>
#include <string>
#include <fstream>
#include <iterator>
#include <vector>
#include <algorithm>
#include <windows.h>

using namespace std;

int main() {

const off_t Size = 4;
unsigned char before[Size] = { 0x12, 0x34, 0x56, 0x78 };
unsigned char  after[Size] = { 0x90, 0xAB, 0xCD, 0xEF };

        vector<char> bytes;
        {
                ifstream iFilestream( "numbers.exe", ios::in|ios::binary );
                istream_iterator<char> begin(iFilestream), end;
                bytes.assign( begin, end ) ;
        }

        vector<char>::iterator found = search( bytes.begin(), bytes.end(), before, before + Size );
        if( found != bytes.end() )
        {
                copy( after, after + Size, found );
                {
                        ofstream oFilestream( "numbers-modified.exe" );
                        copy( bytes.begin(), bytes.end(), ostream_iterator<unsigned char>(oFilestream) );
                }
        }
return 0;
}

Dzięki,
J.

Spróbuj z tym iteratorem:

class istream_fwd_iterator 
	:public iterator<forward_iterator_tag,char,int,char,char>
{
private:
	typedef istream_fwd_iterator	this_type;

	std::istream*	m_istr;
	char			m_val;
	int				m_pos;

	void get_value()
	{
		if(!m_istr)return;
		if(!(*m_istr))m_istr->clear();
		int tmp = m_istr->tellg();
		if(tmp != m_pos) m_istr->seekg(m_pos);
		if(m_istr->read(&m_val,1))
		{
			m_pos = m_istr->tellg();
		}
		else
		{
			m_istr = 0;
			m_pos = 0;
		}
	};
public:
	istream_fwd_iterator():m_istr(0),m_pos(0) {}
	istream_fwd_iterator(istream &is)
		:m_istr(&is),m_pos(0)
	{
		get_value();
	}

	bool operator==(const this_type &it)const
	{
		return m_istr == it.m_istr && m_pos == it.m_pos;
	}

	bool operator!=(const this_type &it)const
	{
		return !(*this == it);
	}

	char operator*()const 
	{ 
		return m_val; 
	}

	this_type& operator++()
	{	
		get_value();
		return *this;
	}

	this_type operator++(int)
	{	
		this_type tmp = *this;
		++*this;
		return tmp;
	}

	int pos()const { return m_pos - 1; }
};

tez tak to rozwiazalem na szybko, ale okazalo sie kilkukrotnie wolniejsze przez ciagle tellg(), dlatego nie wrzucalem..
po przemysleniu problemu z niekopiowalnoscia iteratorow, dochodze do wniosku, ze rozwala ona searcha takze z drugiej strony :) wymuszany ciagly skok do przodu zniszczy oryginalna sekwencje poprzez lookahead w wewnetrznej petli:
_FwdIt1 _Mid1 = _First1 // ++Mid1

ahh i jeszcze jedno! koniecznie nalezy dostarczyc dodatkowo specjalizacje std::distance dla fstream<> !
co prawda search potrzebuje go tylko raz na poczatku do prostego usensownienia swojej pracy..
ale domyslny distance bez specjalizacji przesuwa iteratory 'recznie' i zlicza ile minal - pusc go na 3gb pliku! 3e9 iteracji aby policzyc bajty w pliku

Moją intencją nie było pisanie optymalnego rozwiązania, tylko pokazanie Juta jednego z możliwych rozwiązań, które może sobie bez większego wysiłku przetestować. Być może sprosta ono jego oczekiwaniom.

hej, przeciez nic do Ciebie nie mam:) uprzedzam tylko, poniewaz autor mówił ze oryginal "działa bardzo szybko nawet z dużymi plikami" a ciezko zgadnac czy duzy to 300kB czy 2GB. naprawianie iteratora korci, ale, kurcze, przy mieleniu 2GB jesli 'poprawiony' iter ma dzialac kilka razy wolniej (a taka byla moja impl), to w ogole std::search+distance+iter mijaja sie z celem i lepiej wziac fread/f* i memcmp z C :|

Dzięki 0x666, Twoje rozwiazanie działa. Jest tam co prawda mały błąd - wuszukiwanie działa ale zamiana rozpoczyna się od pozycji 1 zamiast od 0 czyli jak znajdzie 12 13 14 15 to podmienia 13 14 15 i to co jest za 15.

Tak czy inaczej dzięki za to.

Juta

Zabrałem się do tego jeszcze raz ale tym razem zrobiłem to za pomocą CreateFileMapping i teraz działa naprawdę przyzwoicie. Pięć plików o wielkości około 2MB każdy jest przerabianych w czasie około 2 sekund podczas gdy z iteratorami zajmowało to około 7 sekund na jeden plik.

Dzięki za pomoc,
Juta