0

I wrote binary parser which reads and prints various info about file. I wish use E32ImageHeader data directly for speed reasons instead function calls.

I pass nullptr initialized variable and after function return variable remains nullptr. In GetFileLayout() gdb prints various field values for iHdr and some correctly other need revision.

Code looks correct but tools print "Header not recognized!!!", binary for test kf__speedups_SDK.pyd take here

main.cpp:

#include <iostream>
#include "e32info.h"
#include "e32parser.h"

using namespace std;

int main()
{
    E32ImageHeader *hdr = nullptr;
    E32Parser *parser = new E32Parser("AlternateReaderRecogE32.dll", hdr);
    parser->GetFileLayout(hdr);
    if(!hdr)
        cout << "Header not recognized!!!\n";
//    E32Info *f = new E32Info("h", "AlternateReaderRecogE32.dll");
//    f->Run();
    cout << "Hello world!" << endl;
    delete parser;
    return 0;
}

binary parser header file e32parser.h:

#ifndef E32PRODUCER_H
#define E32PRODUCER_H

class E32ImageHeader;

class E32Parser
{
    public:
        E32Parser(const char* aFilename, E32ImageHeader *aHdr);
        ~E32Parser();
        void GetFileLayout(E32ImageHeader *result);
    private:
        void ReadFile();

    private:
        E32ImageHeader *iHdr = nullptr;

        const char *iFileName = nullptr;
        char *iBufferedFile = nullptr;
        size_t iE32Lenth = 0;
}

how it works e32parser.cpp:

#include <fstream>
#include <cstring>
#include <cstdlib>

#include "e32common.h"
#include "e32parser.h"

E32Parser::E32Parser(const char* aFilename, E32ImageHeader *aHdr):
    iHdr(aHdr), iFileName(aFilename)
{
}

E32Parser::~E32Parser()
{
    delete iBufferedFile;
}

Read binary file to internal buffer

void E32Parser::ReadFile()
{
    std::fstream fs(iFileName, fs.binary | fs.in);
    if(!fs)
        throw;
    fs.seekg(0, fs.end);
    int lenth = fs.tellg();
    fs.seekg(0, fs.beg);

    iBufferedFile = new char[lenth];
    fs.read(iBufferedFile, lenth);
    fs.close();;
}

void E32Parser::GetFileLayout(E32ImageHeader *result)
{
    ReadFile();

    iHdr = (E32ImageHeader*)iBufferedFile;
    result = (E32ImageHeader*)iBufferedFile;
}
Fedor
  • 21
  • 4
  • Because you are changing only a copy of the pointer variable. Pass it by reference if you want it to be changed. – πάντα ῥεῖ Sep 04 '18 at 22:27
  • Not sure why the downvotes, the question seems fairly good to me, – Tas Sep 04 '18 at 22:28
  • 1
    @Tas not down-voter but: 2 mins worth of debugging, writing the post took longer. Missing "minimal" in [mcve] – Richard Critten Sep 04 '18 at 22:32
  • `result = (E32ImageHeader*)iBufferedFile;` changes the parameter, but since the parameter is not a reference, it only changes it locally. It does not change the caller's argument. – Eljay Sep 04 '18 at 22:33
  • 1
    Btw, you may get rid of all that `new` with `std::vector` and/or `std::unique_ptr`/value. – Jarod42 Sep 04 '18 at 22:33
  • When passing a pointer what's pointed at is passed by reference. The pointer itself is still passed by value. – user4581301 Sep 04 '18 at 22:35
  • 1
    @RichardCritten disagree a little. The writer knew exactly where the problem was and what the problem was. They just didn't know why. Debugging doesn't usually help with a problem like that. They could have reduced to an MCVE though. – user4581301 Sep 04 '18 at 22:37
  • Unrelated (and probably a future bug) Note: `E32Parser` does not observe [The Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and is vulnerable to bad behaviour when copied deliberately or by accident. [More information on the Rule of Three and Friends](https://en.cppreference.com/w/cpp/language/rule_of_three). – user4581301 Sep 04 '18 at 22:40
  • When you do `int i = 0; doSomething(i);` why does i remain 0 after initialisation? – user253751 Sep 04 '18 at 23:19

1 Answers1

0

Your issue, as pointed out in the comments, is that you are modifying a COPY of the pointer that you pass in. Consider this small example:

void modify(int* p) { p = new int{10}; }

int main()
{
    int* n = nullptr;
    modify(n);
    if (n)
        std::cout << *n;
}

The output of this program is nothing (and it leaks memory). modify doesn't change the original value, it changes a copy of n. The fix to this, and your program, is to pass the pointer by reference.

void modify(int*& p) { p = new int{10}; }

and

void E32Parser::GetFileLayout(E32ImageHeader*& result)

However, I'd argue a better fix is just return the pointer, to prevent this issue:

E32ImageHeader* E32Parser::GetFileLayout()
{
    ...
    iHdr = (E32ImageHeader*)iBufferedFile;
    return iHdr;
}

But since iHdr is passed in to E32Parser, you could change it to be a reference instead

Tas
  • 7,023
  • 3
  • 36
  • 51
  • Thanks a lot! E32ImageHeader* E32Parser::GetFileLayout() is more meaningful and simple variant! – Fedor Sep 05 '18 at 03:02