3

[SOLVED] I changed the char variables to a std::string array, where I push float variables to the array. After all this you can convert the std::string to a const char*

I'm making client to a server. And I want to send data to the server, but when I run this code I am getting 4.5 twice instead of 180 & 4.5. I am using chars, because the send function that I am using only supports chars.

Emptying the char (doesn't work) Printed the value before the if statement ends (This shows that the value is correct), but when the for loop loops the second time it is 4.5 instead of 180.

float an = 180;
float dis = 4.5;

for (int i = 0; i < 1; i++) {
    set.measurements.push_back(an); 
    set.measurements.push_back(dis);
}

for (int i = 0; i < set.measurements.size(); i++){
    sprintf(values, "%0.1f", set.measurements[i]);
    if (i == 1 && i != 0){
         std::cout << "{ 4.5 }" << '\n'; // expected output
         char *dist = new char[10];
         dist = values; // <-- Value is 4.5
         meas.push_back(dist); 
         // This is 4.5 but the angle doesn't stay 180
         delete dist;
    }
    if (i == 0 && i != 1){
        std::cout << "{ 180 }" << '\n'; // Expected output
        char *ang = new char[10];
        ang = values; // <-- Value is 180 here
        meas.push_back(ang); 
        delete ang;
    }
}

for (int i = 0; i < 2; i++) {
  std::cout << &*meas[0] << '\n';
}

I expect the output to be 180 & 4.5, but the actual output is 4.5 & 4.5

Syscall
  • 19,327
  • 10
  • 37
  • 52
mHvNG
  • 671
  • 5
  • 22
  • 1
    You're going to need to present the [mcve] you've been debugging with. – Lightness Races in Orbit Jun 18 '19 at 11:47
  • 1
    `ang = values;` just causes you to leak that `new` allocated memory, and then you are trying to delete `values` instead. *After* you pushed back the pointer to it, that is (but without seeing the rest of the code, who knows what `meas` is and what it does with the pointer anyway). – Blaze Jun 18 '19 at 11:47
  • 1
    `if (i == 1 && i != 0)` What's the point in having both conditions? – Lightness Races in Orbit Jun 18 '19 at 11:47
  • 2
    TL;DR: Do not use `new` and `delete` in C++. Your code is full of undefined behavior. If you need to allocate objects on the heap, use `std::shared_ptr`. Also, use `std::string` instead of `char*`. If you need it as a `char*`, it has a `c_str()` member function. – Nikos C. Jun 18 '19 at 11:50
  • There is no point in having both conditions, it was just one of the many (stupid) tests i did to look if i did something wrong. – mHvNG Jun 18 '19 at 11:51
  • Well, your biggest mistake is deleting dist over and over again cause your new allocation right before vanishes – Narase Jun 18 '19 at 11:53
  • @Blaze Meas is just an empty vector, but i will try your answer and Nikos – mHvNG Jun 18 '19 at 11:55
  • @NikosC. I didn't know about the c_str function – mHvNG Jun 18 '19 at 11:56
  • you should include your whole code. the allocation of `values` is still missing. And if `values` is a vector `sprintf(values,` is going to do some crazy stuff. – Tarick Welling Jun 18 '19 at 11:57
  • 1
    May I recommend you [a good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) for learning C++? Coding by guessing may work for some languages, but is terrible way to learn C++, and it will bite you at every other line. – Yksisarvinen Jun 18 '19 at 11:58
  • @mHvNG what kind of vector? If it's a vector of `char*` then it's a problem if you just `delete` that memory afterwards. If it's a vector of `std::string` then that's one issue less because an `std::string` will be constructed with a copy of the string that the `char*` represents. This is why you should post your whole code (or rather a minimal reproducible example), not just a snippet. – Blaze Jun 18 '19 at 11:58
  • A Tour of C++ (Bjarne Stroustrup) seems like a great fit for you @mHvNG. – Tarick Welling Jun 18 '19 at 11:59
  • @TarickWelling I never said values is a vector, i said Meas is a vector. Values is an empty char[] and in the sprintf function I am pushing the 180 and 4.5 values to it. – mHvNG Jun 18 '19 at 12:00
  • first point still stands, you should include a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) which we can compile ourselves without guessing how some variables are declared – Tarick Welling Jun 18 '19 at 12:02
  • @TarickWelling i edited my code with everything you need. – mHvNG Jun 18 '19 at 12:09
  • @TarickWelling Its not a secret that I am not the best at C++. But i am trying to learn what my problem is, and learn from it for the next time – mHvNG Jun 18 '19 at 12:12
  • My recommendation for 'A Tour of C++ (Bjarne Stroustrup)' is a genuine recommendation. It is available online, fairly cheap, created by the creator of C++ and has a great writing style for learning all the needed details. If you really want the best version you should take the newest edition: 2. It is also great for new informatics students with some programming experience. – Tarick Welling Jun 18 '19 at 12:17
  • @TarickWelling Thanks for the tip, its more the fact that I am maybe not that fairly new to C++. But this is the first time going a bit deeper into C++. But i will have a look! – mHvNG Jun 18 '19 at 12:34

1 Answers1

4
char *dist = new char[10];

This allocates a new char array, and sets dist to point to it.

dist = values; // <-- Value is 4.5

The next line immediately replaces the distpointer to the new-ed buffer, leaking this memory. This now sets dist to point to some other buffer (presumably), instead.

meas.push_back(dist); 

This now adds the pointer to the values object into the vector. Each iteration of the loop, therefore, ends up adding a pointer to the same values buffer into the vector, and that's what you're seeing, in the end.

delete dist;

This is still pointing to values. From the context, it is unlikely that values was originally created with new, and even if it was, each iteration of the loop will end up deleteing the same pointer. Either way, this is undefined behavior, and with enough looping, your entire program will likely crash.

The second if statement repeats all of the above bugs.

Since your intent is, apparently, to write C++ code, you should actually use the C++ library, namely the formatted output operators, and std::strings, that will correctly allocate and manage all the memory for you, eliminating these kinds of common, logical bugs.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • Thanks for the response! The reason I used char was because I am using it in a function where it needed a char, but as someone said as a comment there is a c_str function. – mHvNG Jun 18 '19 at 12:16
  • Please remember that `c_str()` returns a `const CharT*` (`const char*`) and can't be used for writing to the `std::string` – Tarick Welling Jun 18 '19 at 12:20