0

I recently have to start working with c++ and having a lot of problems. my problem here is I need some functionality which are not supported by the same version of c++. For example the following code read a two column file, tokenize it, and store into dictionary :

map<int,int>  ReadTables::lookupTable(string fpath) {
    map<int,int> lookup;    
    ifstream in;
    in.open(fpath.c_str());
    if ( !in.good()) {
        cout << "ERROR: Opening file failed.\n";
        exit (EXIT_FAILURE);
    }
    string line;
    const char delimiter[] = " ";
    vector<string> tokens;
    while (!in.eof()){
        getline(in,line);
        tokens = tokenize( line, delimiter );
        lookup[ atoi(tokens[0].c_str())] = atoi(tokens[1].c_str()); 
        //lookup[ stoi(tokens[0])] = stoi(tokens[1]);   
    } 

This works fine as long as I don't use -std=c++0x flag. When I use this version I got:

 *** Break *** segmentation violation


===========================================================
There was a crash.
This is the entire stack trace of all threads:
===========================================================
#0  0x00007fc4782270ee in waitpid () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007fc4781b9e8e in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007fc47cb0a98e in TUnixSystem::StackTrace() () from /usr/local/lib/root/libCore.so
#3  0x00007fc47cb0a223 in TUnixSystem::DispatchSignals(ESignals) () from /usr/local/lib/root/libCore.so
#4  <signal handler called>
#5  0x00007fc478c64050 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x000000000040a83c in ReadTables::lookupTable (this=0x616500, fpath=...) at ../src/ReadTables.cpp:59
#7  0x000000000040ffd8 in main () at ../src/read.cpp:37
===========================================================


The lines below might hint at the cause of the crash.
If they do not help you then please submit a bug report at
http://root.cern.ch/bugs. Please post the ENTIRE stack trace
from above as an attachment in addition to anything else
that might help us fixing this issue.
===========================================================
#5  0x00007fc478c64050 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x000000000040a83c in ReadTables::lookupTable (this=0x616500, fpath=...) at ../src/ReadTables.cpp:59
#7  0x000000000040ffd8 in main () at ../src/read.cpp:37
==========================================================

I also realized in c11 I should use stoi which again gives this error :

 *** Break *** segmentation violation



===========================================================
There was a crash.
This is the entire stack trace of all threads:
===========================================================
#0  0x00007f26561b50ee in waitpid () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007f2656147e8e in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007f265aa9898e in TUnixSystem::StackTrace() () from /usr/local/lib/root/libCore.so
#3  0x00007f265aa98223 in TUnixSystem::DispatchSignals(ESignals) () from /usr/local/lib/root/libCore.so
#4  <signal handler called>
#5  0x00007f2656bf2050 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x000000000040ac44 in std::stoi (__str=..., __idx=0x0, __base=10) at /usr/include/c++/4.4/bits/basic_string.h:2567
#7  0x000000000040a8e6 in ReadTables::lookupTable (this=0x616500, fpath=...) at ../src/ReadTables.cpp:62
#8  0x0000000000410178 in main () at ../src/read.cpp:37
===========================================================


The lines below might hint at the cause of the crash.
If they do not help you then please submit a bug report at
http://root.cern.ch/bugs. Please post the ENTIRE stack trace
from above as an attachment in addition to anything else
that might help us fixing this issue.
===========================================================
#5  0x00007f2656bf2050 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x000000000040ac44 in std::stoi (__str=..., __idx=0x0, __base=10) at /usr/include/c++/4.4/bits/basic_string.h:2567
#7  0x000000000040a8e6 in ReadTables::lookupTable (this=0x616500, fpath=...) at ../src/ReadTables.cpp:62
#8  0x0000000000410178 in main () at ../src/read.cpp:37
===========================================================

What I am missing here? And is there suggestion about which version to use?

SingerOfTheFall
  • 27,171
  • 7
  • 60
  • 100
Moj
  • 4,733
  • 2
  • 19
  • 33
  • 1
    My guess? Your code has undefined behaviour. – R. Martinho Fernandes Aug 22 '13 at 13:32
  • 3
    On a potentially unrelated note, `while (!in.eof())` [is a broken read loop](http://stackoverflow.com/a/5605159/46642). Forget the source that taught you that (any minimally decent learning material would get this right; it's not that hard), and pick [a better one](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – R. Martinho Fernandes Aug 22 '13 at 13:32
  • You're already using C++11 with `-std=c++0x`, so use something like `std::stoi` that provides error checking if you plan on continued use of that. – chris Aug 22 '13 at 13:33
  • 2
    Just a guess, `tokens[0]` or `tokens[1]` may not be valid indexes. Check the size of the vector returned by `tokenize`. – Jesse Good Aug 22 '13 at 13:36
  • 1
    Looks like you are passing an invalid string to `atoi` or `stoi` - my guess is that `tokens[0]` or `tokens[1]` is invalid - possibly related to the `while(!in.eof())`, as you'd be processing "empty data" in that case. – Mats Petersson Aug 22 '13 at 13:38
  • @R.MartinhoFernandes ,Thanks for the tip. That was my problem – Moj Aug 22 '13 at 14:07

2 Answers2

3

If tokenize fails to return a vector with at least two elements your code will access past the end of the vector and run into undefined behavior. One such undefined behavior is that your program appears to work in C++98 and fails dramatically in C++11 compilation.

Mark B
  • 91,641
  • 10
  • 102
  • 179
  • 2
    To be clear, your program ISN'T working in C++ 98 - it's failing silently. 'tokens' has the potential to be empty, and you need to check for that before you start accessing it's elements. – Michael Kohne Aug 22 '13 at 13:48
  • Thanks . The problem was indeed in ` while (!in.eof())`. so changed it to `in>>key>>value` and `lookup[ stoi(key)] = stoi(value)` . Now it works fine. even no need to `tokenize` – Moj Aug 22 '13 at 14:05
  • If the values in the file are stored as integers, there is no need to read them in as strings and then convert them. Just read them in as integers. `int key, value; {loop} in >> key >> value; lookup[key] = value; {/loop}`. No need to call `stoi`. – Zac Howland Aug 22 '13 at 14:11
1

You can greatly simplify your code:

std::map<int,int>  ReadTables::lookupTable(const std::string& fpath) 
{
    map<int,int> lookup;    
    std::ifstream fin(fpath.c_str());
    if (!fin.good()) 
    {
        cout << "ERROR: Opening file failed.\n";
        exit(EXIT_FAILURE); // should probably throw an exception here instead of exiting the program
    }

    // load all the integers from the file
    std::vector<int> v;
    std::copy(std::istream_iterator<int>(fin), std::istream_iterator<int>(), std::back_inserter<vector<int>>(v));
    assert(v.size() % 2 == 0); // assert that the file loaded key/value pairs
    for (i = 0; i < v.size(); i += 2)
    {
        lookup[v[i]] = v[i + 1];
    }

    fin.close();
    return lookup;
}

I'd have to do some digging, but you can probably simplify it even more by tweaking the std::copy call to load directly into the std::map.

Zac Howland
  • 15,149
  • 1
  • 23
  • 37