0

I am trying to do some file reading with C++ in Ubuntu 16.04 (GCC&G++ 5.4 and CMake 3.5.1). The test file (named 123.txt) have only a line words just like this:

Reprojection error: avg = 0.110258   max = 0.491361

I just want to get the avg error and max error. My method is to get a line and put them into a std::string and use string::find. My codes are very easy just like this:

#include <iostream>
#include <string>
#include <stdio.h>

using namespace std;

int main()
{
    FILE *fp = fopen("123.txt", "r");
    char tmp[60];
    string str;
    fgets(tmp, size_t(tmp), fp);
    fclose(fp);
    cout << tmp << endl;
    str = tmp;
    cout << str.size() << endl;
    size_t avg = str.find("avg");
    size_t max = str.find("max");
    cout << avg << endl;
    cout << max << endl;
}

I can use g++ to compile it successfully. But I meet a strange issue.

When I first run it in the command, it will get the right result:

Reprojection error: avg = 0.110258   max = 0.491361

52
20
37

If I run codes again, it will go wrong sometimes just like this:

p
2
18446744073709551615
18446744073709551615

The "p" is a disorderly code which can not be shown correctly in the command. I am not good at C++ and feel confused about it. Is there someone who can say something? Thank you!

anastaciu
  • 23,467
  • 7
  • 28
  • 53
Yancen BOB
  • 11
  • 3
  • Always check the return from the `fopen("123.txt", "r")` call, to make sure the file was successfully opened. – Adrian Mole May 20 '20 at 09:15
  • I very much dout that `size_t(tmp)` is not a compilation error. – molbdnilo May 20 '20 at 09:16
  • `tmp` is not terminated with a zero, so your program has undefined behaviour. – molbdnilo May 20 '20 at 09:17
  • @molbdnilo, amazingly it's not, it produces a random number but no warnings and no errors. – anastaciu May 20 '20 at 09:23
  • You probably want to write `memset(tmp, 0, sizeof(tmp)); fgets(tmp, sizeof(tmp)-2, fp);` instead. What `size_t(tmp)` does, is it converts the address where tmp starts to a size_t and passes that to `fgets`. That address can be a very large number and you probably trash the stack. – DNT May 20 '20 at 09:29

1 Answers1

0

The expression

fgets(tmp, size_t(tmp), fp);

is ill-formed, size_t(tmp) will not work as you expect, you need sizeof(tmp).

The 52 value you get is because fgets consumes the \n character and this is counted too, actually the string has 51 characters counting with spaces.

That said, in this case you can use better C++ tools to replace the C ones you are using, fopen can be replaced by using the fstream library, fgets can be replaced by getline.

Something like:

#include <iostream>
#include <string>
#include <fstream>

int main()
{
    std::ifstream fp("123.txt"); //C++ filestream

    if (fp.is_open()) {//check for file opening errors

        std::string str;
        std::getline(fp, str); //C++ read from file
        fp.close();
        std::cout << str << std::endl;
        std::cout << str.size() << std::endl;
        size_t avg = str.find("avg");
        size_t max = str.find("max");
        std::cout << avg << std::endl;
        std::cout << max << std::endl;
    }
    else{
        std::cerr << "Couldn't open file";
    }
}

Note that I dind't use using namespace std;, this is for a reason, it's not a good practice, you can check this thread for more details.

anastaciu
  • 23,467
  • 7
  • 28
  • 53
  • 1
    How I hate std::cout, in 99% of cases I still prefer printf when writing C++ code. https://gist.github.com/bkaradzic/2e39896bc7d8c34e042b – Konrad May 20 '20 at 09:55
  • @Konrad The issue here is not `std::cout`, that is already being used by the OP, the real issues are line reading and file opening, though your link has an interesting opinion that I, at times, can share, in this case I don't agree, I believe `std::getline` and `std::ifstream` are effectively better than their C counterparts. As for `std::cout`, a real unbiased analysis: https://stackoverflow.com/a/20238349/6865932 – anastaciu May 20 '20 at 10:05
  • I know. I just wanted to know your opinion. I agree about std::getline and std::ifstream. I just commented so maybe OP will prefer using `printf` in the future, or something else that doesn't require using `<<` operator like spdlog ;) so it will be easier and more comfortable to write and read. Other languages usually provide something like `print` which is much much better and more elegant, e.g. in rust – Konrad May 20 '20 at 10:17
  • @Konrad, it's a good comment yours, I did change my answer based on it, I really don't want to pass the message that you can't mix C and C++, if a C tool is better for a given task I encourage it's use, and `printf` is a really good example, though I wouldn't go so far as to say 99% of the times :) – anastaciu May 20 '20 at 10:22