File einlesen mit Hex Zahlen

lord.hong

Hallo an alle,

ich habe eine File vorliegen, was nach folgendem Muster aufgebaut worden ist:

abc:def:pkt:1000:0x10 0x10 0x10 usw

Der Wert nach "pkt:" gibt die Anzahl der Zeichen wieder - in diesem Fall 1000 Bytes.

Ich habe die Routine in einen if/else Anweisung aufgeteilt. Der if Zweig stellt fest, dass das Array 1000 Byte groß sein soll und sichert den ersten Hex-Wert. Anschließend werden alle Hex-Wert eingelesen über den else Zweig bis ich wieder auf ein 'a' treffe und wieder ein neues Array mit vielleicht einer anderen Größe treffe.

Hier der Code:

std::ifstream srcFile;
  srcFile.open("input.txt", std::ios_base::in);

  unsigned char* pPaket = 0;
  unsigned int uiCurrentIndex = 0;
  unsigned int uiPaketSize = 0;

  std::vector<unsigned char*> v;

  while (!srcFile.eof())
  {
    std::string sTemp;
    srcFile >> sTemp;

    if (sTemp.find_first_of('a') != std::string::npos)
    {
      pPaket = 0;
      uiCurrentIndex = 0;
      uiPaketSize = 0;

      // abc:def:ghi:pktSize:10000:0x00
      size_t sEndIndex = sTemp.length();
      size_t sBeginIndex = sTemp.find_last_of(':');
      std::string sFirstHex = sTemp.substr((sBeginIndex + 1), sEndIndex);

      sEndIndex = sBeginIndex - 1;
      sBeginIndex = sTemp.rfind(':', sEndIndex);

      std::string sPaketSize = sTemp.substr((sBeginIndex + 1), (sEndIndex - sBeginIndex));
      uiPaketSize = atoi(sPaketSize.c_str());
      assert(uiPaketSize > 0);

      pPaket = new unsigned char[uiPaketSize];
      assert(uiCurrentIndex == 0);

      int i = 0;
      sscanf(sFirstHex.c_str(), "%lx", &i);
      std::memcpy(&pPaket[uiCurrentIndex], &i, sizeof(unsigned char));
      ++uiCurrentIndex;
      assert(uiCurrentIndex == 1);

      v.push_back(pPaket);
    }
    else if (sTemp.find_first_of('0') != std::string::npos)
    {
      assert(pPaket != 0);

      int i = 0;
      sscanf(sTemp.c_str(), "%lx", &i);
      std::memcpy(&pPaket[uiCurrentIndex], &i, sizeof(unsigned char));
      ++uiCurrentIndex;
    }
  }

  srcFile.close();

Allerdings wollte ich nun wissen, ob die Rountine C++ freundlich ist.

Zweifel habe ich inzwischen wegen folgenden Aufruf:

sscanf(sTemp.c_str(), "%lx", &i);
      std::memcpy(&pPaket[uiCurrentIndex], &i, sizeof(unsigned char));

sscanf kommt nach meinem Verständnis mehr aus C. Außerdem warnt der Compiler vor der Benutzung. Gibt es eine Alternative? Ich hatte große Problem den Hex-Wert in ein char zu bekommen. Der Hex-Wert "0x01" wird in vier Character geschrieben, was gar nicht so leicht war, den Wert in ein Byte (unsigned char) umzuwandeln. Gibt es eine elegantere Möglichkeit?

Was auch nicht so leicht war, war den eingelesenen String "bc:def:ghi:pktSize:10000:0x00" zu parsen. ich bin nur an den letzten zwei Werten interessiert. Gibt es in C++ (std::string) eine Möglichkeit mit split(':'). Ich dachte, dann hätte ich ein Array/Vektor und könnte die zwei letzten Werten auslesen. (wie in Ruby oder Phyton)

Für ein Code Review und Kritik wäre ich dankbar.

Gruß
Markus

Werner Salomon

lord.hong schrieb:

Zweifel habe ich inzwischen wegen folgenden Aufruf:
sscanf(sTemp.c_str(), "%lx", &i);
      std::memcpy(&pPaket[uiCurrentIndex], &i, sizeof(unsigned char));
sscanf kommt nach meinem Verständnis mehr aus C. Außerdem warnt der Compiler vor der Benutzung. Gibt es eine Alternative? Ich hatte große Problem den Hex-Wert in ein char zu bekommen. Der Hex-Wert "0x01" wird in vier Character geschrieben, was gar nicht so leicht war, den Wert in ein Byte (unsigned char) umzuwandeln. Gibt es eine elegantere Möglichkeit?

Hallo Markus,
ja - eine simple Zuweisung - ggf. inklusive cast

pPaket[uiCurrentIndex] = static_cast< unsigned char >( i );

Man könnte vorher noch abfragen, ob das i auch in ein unsigned char hineinpasst ..

if( i <= std::numeric_limits< unsigned char >::max() )
        // .. ok passt

lord.hong schrieb:

Für ein Code Review und Kritik wäre ich dankbar.

.. Du hast es so gewollt

zunächst mal ein paar Basics:

- sscanf und memcpy sind nicht C++-like - Lösung s.o.
- atoi genauso - komme ich nachher noch zu
- es ist in C++ üblich, Variablen erst dann zu deklarieren, wenn man sie benötigt. In diesem Fall kann man 'uiPaketSize' erst mit Aufruf von atoi anlegen.

unsigned int uiPaketSize = atoi(sPaketSize.c_str());

- Die Lösung mit einem std::vector von Pointern ist in in so weit ungünstig, da Du Dich später selber um das Löschen des Speichers kümmern musst. Es wäre durchaus in Ordnung die Elemente von 'v' selbst zu Containern zu machen.

std::vector< std::vector< unsigned char > > v;

- Die Schleife, die zum einen über alle Hexwerte und zum anderen auch über alle Arrays von Hex-Werten läuft, macht das ganze zumindest schwierig zu verstehen. Die Lösung ist im Prinzip ok, aber da es keinen erkennbaren Vorteil bringt, würde ich doch zwei ineinander geschachtelte Schleifen wählen.

- Die Sequenz

{
  std::ifstream srcFile;
  srcFile.open("input.txt", std::ios_base::in);
  // ...
  srcFile.close();
}

ist so gar nicht notwendig. C++-like ist ein einfacheres

{
  std::ifstream srcFile("input.txt", std::ios_base::in); // srcFile mit dem Anlegen öffnen
  // ...
} // close macht der Destruktor

.. und jetzt zum Schwierigsten - dem Lesen aus einem Stream.
Das von Dir gewählte Vorgehen - erst in einen String zu lesen und dann den String auseinander nehmen - ist weit verbreitet; resultiert aber letztlich nur aus dem Unwissen den std::istream richtig zu nutzen.
Man braucht diesen Umweg über den string nicht - auch hier nicht, obwohl das Format Deiner Datei durchaus etwas schwierig ist. Es gilt grundsätzlich die Regel: Lese das, was Du auch haben willst. Also wenn Du Zahlen lesen willst, so tue es auch. Den Vorspann kannst Du mit der Methode ignore überspringen.

const std::streamsize ALL = std::numeric_limits< std::streamsize >::max(); // .. brauchen wir öfter (s.u.)
    std::vector< std::vector< unsigned char > > vv;
    std::ifstream srcFile( "input.txt", std::ios_base::in );
    while( srcFile.good() )  // !fail() && !eof()
    {
        for( int i = 0; i < 3; ++i )        // 3*':' überlesen
            srcFile.ignore( ALL, ':' );

damit steht der Lesezeiger vor der Paketgroesse, die wir jetzt einfach inklusive des folgenden Doppelpunktes lesen

std::size_t PaketSize;
        char doppelpunkt;
        if( !(srcFile >> PaketSize >> doppelpunkt ) || PaketSize == 0
            || doppelpunkt != ':' )
        {
            srcFile.setstate( std::ios_base::failbit );
            break;
        }

und wenn es schiefgehen sollte - z.B. kein Doppelpunkt folgt - so brechen wir gleich ab und setzen den Stream auf Fehler.
Es folgt die Schleife über 'PaketSize' Hex-Werte

std::vector< unsigned char > v( PaketSize );
        srcFile >> std::hex;
        for( std::vector< unsigned char >::iterator dst = v.begin()
            ; dst != v.end(); ++dst )
        {
            unsigned int zahl;
            if( !(srcFile >> zahl) )   // liest auch 0x-Prefix, da .. >> std::hex
                break;
            if( zahl > std::numeric_limits< unsigned char >::max() )
            {
                srcFile.setstate( std::ios_base::failbit );
                break;
            }
            *dst = static_cast< unsigned char >( zahl );
        }

zunächst wir ein Container angelegt - mit der richtigen Größe - und in der Schleife wird er gefüllt. Vorher wird das Hex-Flag gesetzt; und damit können auch die Ausdrücke wie "0x10" als Zahlen gelesen werden. Und am Ende einfach die Zuweisung (s.o.).

Ist der Container voll ..

if( srcFile )   // noch alles ok?
        {   // ohne Kopie übernehmen
            vv.push_back( std::vector< unsigned char >() );
            swap( vv.back(), v );
        }
    } // Ende while

.. so wird der Container nur dann übernommen, wenn 'srcFile' noch ok ist. Dann kann man sicher sein, dass alle Elemente auch belegt worden sind. Hier hätte auch ein einfaches

if( srcFile )   // noch alles ok?
        {
            vv.push_back( v );
        }

genügt, aber es gibt 'ne Menge Leute, die das Kopieren eines ganzen Containers mehr fürchten, als der Teufel das Weihwasser. Der swap-Trick tauscht die Vektoren aus, ohne das die Elemente kopiert werden.

Das unmittelbaren Einlesen hat dann auch den Vorteil, dass Du nicht zwischen dem ersten und den folgenden Hexwerten unterscheiden brauchst und das atoi wird ganz von allein überflüssig.

Gruß
Werner

Werner Salomon

Hallo Markus,
.. ich war noch nicht fertig; es war schon spät gestern.

Mit der oben angegebenen Lösung befindet sich noch alles in einer Funktion, was i.A. nicht sinnvoll ist. Du willst ja mit diesem 'Paket' von Bytes nach dem Einlesen noch irgendwas machen. Damit Du damit 'so richtig hantieren' kannst, sollte man daraus eine Klasse machen. Da ich nicht weiß, was es werden soll, nenne ich sie einfach mal X. Jedes X-Objekt enthält ein Paket Bytes.

Also am Ende solltest Du einen vector von X-Objekten haben:

class X;
  std::vector< X > v;

Für das Einlesen ist das Ziel, folgendes hinschreiben zu können:

std::ifstream srcFile( "input.txt", std::ios_base::in );
    std::vector< X > v( (std::istream_iterator< X >( srcFile )), std::istream_iterator< X >() );
    // usw. nutze v

Damit das funktioniert benötigen wir eine Funktion zum Einlesen von X

class X
{
public:
    // .. weitere Methoden

    friend std::istream& operator>>( std::istream& in, X& x );
private:
    std::vector< unsigned char > m_paket;
};

In dieser friend-function steht jetzt im Prinzip der Code aus dem Innern der while-Schleife meines vorherigen Postings.

Einige Dinge kann man jedoch vorher herausnehmen, da sie allgemein verwendbar sind.

Das ist zum einen das Einlesen eines bestimmten Zeichens - also der Doppelpunkt. Das gab's schon mal einen Thread wo das Einlesen eines Trenners beschrieben ist.

// Anwendung: in >> trenner<'#'>; // überliest ein #
template< char D >
std::istream& trenner( std::istream& in )
{
    char t;
    if( in >> t && t != D )
        in.setstate( std::ios_base::failbit );
    return in;
}

das nächste wäre das Einlesen eines Bytes als Integer. Dafür kann man sich eine kleine Struktur bauen

// Anwendung: 
//      unsigned char byte;
//      if( in >> readByte( byte ) )
//          // 'byte' richtig gelesen 
struct readByte
{
    explicit readByte( unsigned char& c ) : m_c( c ) {}
    friend std::istream& operator>>( std::istream& in, const readByte& rb )
    {
        unsigned int i;
        if( in >> i )
        {
            if( i <= std::numeric_limits< unsigned char >::max() )
                rb.m_c = static_cast< unsigned char >( i );
            else
                in.setstate( std::ios_base::failbit );
        }
        return in;
    }
private:
    unsigned char& m_c;
};

bleibt noch das Einlesen des Vorspanns bzw. Prefix, was man z.B. als Manipulator in eine statische Funktion der Klasse X auslagern könnte. Es ist ja schließlich Sache von X, wie X gelesen wird.

class X
{
    // .. weitere Methoden

private:
    static std::istream& prefix( std::istream& in )
    {
        for( int i = 0; i < 3; ++i )
            in.ignore( std::numeric_limits< std::streamsize >::max(), ':' );
        return in;
    }
    // --   Member
    std::vector< unsigned char > m_paket;
};

Mit diesen drei Helferlein wird die Einlese-Funktion von X recht übersichtlich

std::istream& operator>>( std::istream& in, X& x )
{
    using namespace std;
    size_t PaketSize;
    if( in >> X::prefix >> dec >> PaketSize >> trenner<':'> )
    {
        if( PaketSize > 0 )
        {
            in >> hex;
            vector< unsigned char > v( PaketSize );
            unsigned char c;
            for( vector< unsigned char >::iterator i = v.begin(); i != v.end() && in >> readByte( c ); ++i )
                *i = c;
            if( in )                // nach korrektem Einlesen übernehmen
                swap( x.m_paket, v );
        }
        else
            in.setstate( ios_base::failbit );   // PaketSize war == 0
    }
    return in;
}

Bleibt noch zu erwähnen, dass man die Format-Flags des Streams, die mit dec und hex verstellt worden sind, beim Verlassen der Funktion in den alten Zustand zurückstellen sollte. Dafür gibt es aber was Fertiges bei boost.io state savers. Daher spare ich mir da weitere Erläuterungen.

Das ganze würde ich jetzt als C++-like bezeichnen

Gruß
Werner

Th

Werner, klasse!!! Selbst ich (als professioneller Software-Entwickler) kann immer noch etwas von dir lernen - insbesondere die Einfachheit des Designs deiner Klassen bzw. Hilfsfunktionen gefällt mir).

Wenn auch nur die Hälfte der C++ Programme so wie deine Programme aussehen würden, das wäre schon super, aber leider verwenden zu viele Leute immer noch den alten C-Style.

Vllt. solltest du mal ein C++ Tutorial verfassen?

*D*Evil

Joa man muss schon zugeben ... Werner schreibt doch immer sehr sauberen C++-Code Respekt

lord.hong

Hallo Werner,

erstmal Danke, dass du dir die Zeit genommen hast und auch ein paar Punkte angespricht, die ich anscheinend verdrängt habe. Zuerst kann ich erstmal nur auf deinen ersten Eintrag eingehen, da ich ihn zum Teil verstehe.

ja - eine simple Zuweisung - ggf. inklusive cast
pPaket[uiCurrentIndex] = static_cast< unsigned char >( i );

Ich habe etwas Zweifel, ob es doch so einfach ist. Es handelt sich nicht um einen Character sondern um vier "0x80". Diese vier Character müssen nun in einen unsigened char umgewandelt werden. So bin ich auf sscanf gestossen.

- sscanf und memcpy sind nicht C++-like - Lösung s.o.
- atoi genauso - komme ich nachher noch zu

Bei sscanf und atoi war mir bekannt, memcpy hatte ich wohl vergessen.

- es ist in C++ üblich, Variablen erst dann zu deklarieren, wenn man sie benötigt. In diesem Fall kann man 'uiPaketSize' erst mit Aufruf von atoi anlegen.

Normalerweise benutze ich keine temporäre Variablen. In dem Fall muss ich aber zu geben, dass der Code nicht aufgeräumt ist und es auch keine Test Cases gibt. Ich war in erster Linie froh, dass ich die Aufgabestellung hinbekommen habe.

- Die Lösung mit einem std::vector von Pointern ist in in so weit ungünstig, da Du Dich später selber um das Löschen des Speichers kümmern musst. Es wäre durchaus in Ordnung die Elemente von 'v' selbst zu Containern zu machen.
std::vector< std::vector< unsigned char > > v;

Bei den Hex-Werten handelt es sich um IP Pakete die an weitere Layer gegeben werden. Der Zeiger auf eine unsigned char Array ist historisch begründet. Ich glaube nicht, dass ich meine Kollegen dazu umstimmen kann. (Mögen anscheinend kein C++ und schon gar nicht die STL )

- Die Schleife, die zum einen über alle Hexwerte und zum anderen auch über alle Arrays von Hex-Werten läuft, macht das ganze zumindest schwierig zu verstehen. Die Lösung ist im Prinzip ok, aber da es keinen erkennbaren Vorteil bringt, würde ich doch zwei ineinander geschachtelte Schleifen wählen.

Habe ich jetzt nicht ganz verstanden. Meine Schleife geht bis zum end-of-file Zeichen. Ich erkenne zur Zeit nicht wozu ich eine weitere Schleife benötige.

Das von Dir gewählte Vorgehen - erst in einen String zu lesen und dann den String auseinander nehmen - ist weit verbreitet; resultiert aber letztlich nur aus dem Unwissen den std::istream richtig zu nutzen.

Ja, da gehöre ich wohl dazu, wenn ich ehrlich bin, habe ich auch eine Lösung mit dem Streams gesucht, aber mir sehr schwer getan.

Zu Ende hin, habe ich immer weniger verstanden. Ich werde nun erstmal versuchen den Code umschreiben wie es von dir vorgeschlagen wurde. Vermutlich muss ich mich auch noch in Streams einlesen, um zu verstehen, was du mir beibringen möchtest.

Aber nochmals Danke! Ich bleibe am Ball!

Gruß
Markus

lord.hong

Super, das ist ein ganz anderer Lösungsansatz.

Zur Zeit wird nur ein IP Paket eingelesen. Beim zweiten Paket ist die Größe zur Zeit immer 65536, aber das werde ich morgen lösen können.

EDIT: Ich glaube ich weiß wieso. Der Stream versucht nun die Größe als Hex-Wert einzulesen, ist es aber nicht.

Eine Zeile ist mir nicht klar geworden:

const std::streamsize ALL = std::numeric_limits< std::streamsize >::max();

Was passiert hier?

Gruß
Markus

*D*Evil

Naja optimiert sollte man schreiben:

const std::streamsize ALL(std::numeric_limits< std::streamsize >::max());

Wobei das haarspalterrei ist
Also ehm damit bekommst du die maximale Anzahl von Zeichen die in einen Stream passen, da dessen Größe durch std::streamsize angegeben wird Guck dir einfach mal das numeric_limits-Template an

lord.hong

Boah, whoh, unglaublich!!! Funktioniert! Liest das ganze File nun ein!

std::size_t uiPaketSize = 0;
char cColon;

inputFile >> std::dec;
if (!(inputFile >> uiPaketSize >> cColon) ||
    (uiPaketSize == 0) || 
    (cColon != ':'))
{
  inputFile.setstate( std::ios_base::failbit);
  break;
}

Hallo Markus,
.. ich war noch nicht fertig; es war schon spät gestern.

Genau und morgen lese ich den zweiten Teil!

Ciao,
Markus

Werner Salomon

lord.hong schrieb:

Der Stream versucht nun die Größe als Hex-Wert einzulesen, ist es aber nicht.

Hallo Markus,

stimmt, das war mein Fehler, später hatte ich ihn noch bemerkt:

Werner Salomon schrieb:

Bleibt noch zu erwähnen, dass man die Format-Flags des Streams, die mit dec und hex verstellt worden sind, beim Verlassen der Funktion in den alten Zustand zurückstellen sollte. Dafür gibt es aber was Fertiges bei boost.io state savers. Daher spare ich mir da weitere Erläuterungen.

@Th, @(D)Evil Danke für Euer Lob

Gruß
Werner

lord.hong

Hallo Werner,

mir gefällt sehr gut die Lösung mit dem failbit im Fehlerfall. Auch finde ich, dass du ein sehr gutes Verständnis für die stl hast. Auf eine derartige Benutzung wäre ich nie gekommen.

Ich habe mir das erste Programm nochmals angeschaut und habe nun auch ein paar Fragen, was mir immer noch nicht klar geworden ist.

Außerhalb der while-Loop hast du ALL deklariert und initialisiert

const std::streamsize ALL = std::numeric_limits< std::streamsize >::max(); // .. brauchen wir öfter (s.u.)
std::vector< std::vector< unsigned char > > vv;
std::ifstream srcFile( "input.txt", std::ios_base::in );
while( srcFile.good() )  // !fail() && !eof()
{
    for( int i = 0; i < 3; ++i )        // 3*':' überlesen
        srcFile.ignore( ALL, ':' );

Warum hast du es nicht in der for-Schleife getan?

for( int i = 0; i < 3; ++i )
    srcFile.ignore( std::numeric_limits< std::streamsize >::max(), ':');

Auch diese Anweisung ist mir nicht ganz klar:

if( !(srcFile >> zahl) )
    break;

Obwohl ich den Ausdruck auch in "The C++ Standard Library" von Josuttis gefunden habe (s. 600) ist mir nicht klar was hier genau passiert. Allerdings auf so eine Art und Weise, dass ich gar nicht weiß, wie fragen soll.

Mir ist nicht klar, wie die Anweisung im if-Block true oder false wird. Ich denke zwar, dass die Anweisung (src >> zahl) wiederrum einen ifstream zurückgibt. Ich weiß auch das operator!() und operator void*() überschrieben worden sind, aber ich kann mir zur Zeit nich erklären, wieso der Wert im guten Fall true ist.

Auch die folgende if-Anweisung konnte ich mir nicht erklären:

if( zahl > std::numeric_limits< unsigned char >::max() )

Einerseits ist mir nicht klar geworden, warum du dich für einen unsigned int für Zahl entschieden hast. Anderseits wollte ich sehen, was in dem Ausdruck rechts steht:

int main(int argc, char* argv[])
{
    std::cout << std::boolalpha;

    std::cout << "max(unsigned char)" << std::numeric_limits<unsigned char>::max() << std::endl;
    std::cout << std::endl;

    return 0;
}

Aber warum auch immer sehe ich nichts. Die Beispiele von Josuttis util/limits1.cpp (short, int, long, float, double, and long double) funktionieren. Vermutlich habe ich auch das mit den numeric_limits nicht richtig verstanden.

Es bleibt zwar nicht mehr viel über, aber den Rest glaube ich habe verstanden. Übrigends fand ich auch interessant, dass wirklich kein close mehr aufgerufen werden muss.

Gruß
Markus

lord.hong

(D)Evil schrieb:

Guck dir einfach mal das numeric_limits-Template an

Oh ja, Templates! Entweder mag man sie, oder nicht! Ich tue mir sehr schwer damit.

Außer MinT und MaxT habe ich sie auch noch nicht benutzt.

Vielleicht kannst du mir aber erklären, was die erste Zeile mir sagen könnte:

template<> 
class numeric_limints<float>

Auch ist alles static und const, aber warum?

static const int min_exponent = -125
...

Auch dieses Beispiel habe ich aus "The C++ Standard Library" und auch den Absatz gelesen, aber das kommt mir alles viel zu schwer vor. Das braucht bestimmt noch Zeit.

Gruß
Markus

*D*Evil

Warum hast du es nicht in der for-Schleife getan?

Weil es dann in jedem Schleifenaufruf ausgeführt wird. Ist eine Sache der Optimierung.

if( !(srcFile >> zahl) )
    break;

Hmm ... operator >> von std::istream(Referenz) liefert dir einen std::istream& zurück. operator! von std::ios(Referenz) gibt dir true zurück, wenn ein fail-, badbit gesetzt wurde.
Folglich wird erst versucht die Zahl einzulesen ... wenn es nicht klappt wird das failbit gesetzt und operator! gibt true zurück. Ausgeschrieben:

srcFile >> zahl;
if (!srcFile) break;

...

if( zahl > std::numeric_limits< unsigned char >::max() )

Weiß war gerade nicht in welchem Kontext die Zeile steht, aber damit kannst du überprüfen, ob die Variable zahl einen zu großen Wert hat, als das sie in den Wertebereich von unsigned char passen würde.

*D*Evil

Vielleicht kannst du mir aber erklären, was die erste Zeile mir sagen könnte:
template<>
class numeric_limints<float>

Hmm ... das war glaube ich sogar Thema ein paar in den letzten Tagen ... der Titel des Themas sah zumindest danach aus
Also es handelt sich dabei um eine spezifizierung des Templates. Wahrscheinlich sieht das Template sonst so aus:

template<typename T>
class numeric_limints

. Wenn jetzt aber std::numeric_limits<float> genommen wird, soll die obere Version spezifizierte Version aufgerufen werden.

Auch ist alles static und const, aber warum?

Ehm naja der Wertebereich eines Datentypes ändert sich ja nicht so schnell D.h. die Werte sind bei den POD fest vorgegeben.