CODEWITHSUNDEEP
c++classoop
Dohn Joe
Dohn Joe

Reputation: 35

Result of my code adds weird characters in console

I've tried to write a program which shows the result of a soccer game, using a class Match in C++. Here is my code:

#include <iostream>
#include <conio.h>
using namespace std;
class Match {
private:
    typedef struct team {
        char name[50];
        int  goal   ;
    }echipa;
    team e1;
    team e2;

public:
    void init_name(char team1_name[], char team2_name[]);
    void init_goal(int g1, int g2);
    void print();
    };
    void Match::init_name(char team1_name[], char team2_name[]) {
        strncpy(e1.name, team1_name, sizeof(team1_name));
        strncpy(e2.name, team2_name, sizeof(team2_name));
    }
    void Match::init_goal(int g1, int g2) {
        e1.goal = g1;
        e2.goal = g2;
    }
    void Match::print() {
        cout << e1.name << " " << e1.goal<< " " << " - " << " " << e2.goal<< " " << e1.name << endl;
    }

void main ()
{
    Match x;
    x.init_name("Team1", "Team2");
    x.init_goal(5, 4);
    x.print();
    getch();
}

In console, instead of Team1 5 - 4 Team2 I have Team╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠♣ 5 - 4 Team╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠♣ I have no idea why these characters ╠♣ appear everytime I run the code, any suggestion on how to fix this?

Upvotes: 0

Views: 490

Answers (5)

JaMiT
JaMiT

Reputation: 16843

Copying a std::string is simpler than using strncpy(), but since you seem to be stuck with the C legacy for now:

The reason to use strncpy instead of strcpy is to prevent buffer overflows. That is, it prevents overwriting the end of the destination buffer. The difference between these two functions lies in the third parameter of strncpy, a parameter that strcpy does not have. This parameter specifies the number of characters the destination buffer can accept. The copy will stop at that point, preventing writes to unavailable memory.

You are using the third parameter to limit the copy based on the source string. This is wrong, as a too-long source string could still overwrite the end of the destination buffer. Plus, your source is null-terminated, so the copy will already stop at the end of the source string if it doesn't stop earlier. Use the size of the destination buffer as the limit, as in

strncpy(e1.name, team1_name, sizeof(e1.name));
//                                  ^^^^^^^

and don't forget to add a null-terminator in case an overflow had happened

e1.name[sizeof(e1.name)-1] = '\0';

One improvement would be to use a symbolic constant for the buffer size so you don't have to make sure that sizeof is using an array instead of the result of decaying to a pointer. (Decaying to a pointer is why your version limited the copy to four characters instead of the six in your strings.) Even better, use std::string instead of char []. The interface is simpler (assign a new value with =) and you are not limited to 49 characters in your team names.

Upvotes: 1

f180362 Asad Ullah
f180362 Asad Ullah

Reputation: 49

        `void init_name(string a, string b);
void init_goal(int g1, int g2);
void print();

};

void Match::init_name(string a, string b) {
strncpy_s(e1.name, a.c_str(),a.size());
strncpy_s(e2.name, b.c_str(),b.size());

}`

use strncy_s instead of strncpy .and pass parameter as string instead of array.

Upvotes: 1

Aykhan Hagverdili
Aykhan Hagverdili

Reputation: 29965

That is not how we write sane C++. There are lots of red flags in your code. In C++, you use a constructor to construct the object and you use std::string to represent text unless you have a good reason not to. You don't need typedef struct. I suggest you learn from a good book. Here's a list of good C++ books. Having said that, here's a sane way of doing what you're doing:

#include <iostream>
#include <string>
#include <utility>

class Match {
 private:
  struct team {
    std::string name;
    int goal;
  };

  team e1;
  team e2;

 public:
  Match(team t1, team t2) 
    : e1{std::move(t1)}
    , e2{std::move(t2)} 
  {}

  void print() const {
    std::cout << e1.name << ' ' 
              << e1.goal << " - " 
              << e2.goal << ' ' 
              << e1.name << '\n';
  }
};

int main() {
  Match x{{"Team1", 5}, {"Team2", 4}};
  x.print();
}

Upvotes: 1

mch
mch

Reputation: 9804

void Match::init_name(char team1_name[], char team2_name[]) {
    strncpy(e1.name, team1_name, sizeof(team1_name));
    strncpy(e2.name, team2_name, sizeof(team2_name));
}

copies 4 or 8 bytes, depending on your system, from team1_name to e1.name. You should use strcpy(e1.name, team1_name); to copy the string.

Upvotes: 3

Mark Ransom
Mark Ransom

Reputation: 308130

strncpy doesn't put a terminating null on a string if the string is longer than the count you've specified, so you're reading a bunch of random memory past the end of the string. The technical term is "undefined behavior".

In addition, as noted in the comments you're using sizeof on a pointer thinking it will return the length of the string. It does not. I think you'll find that it's returning a number much smaller than your string, leading to the problem identified above.

Upvotes: 4

Related Questions