Why didn't work C++ code(strncpy_s)?

Question

I'm C++ beginner.

When I have studied Class of C++, this example code did not work.

This code is interrupted by strncpy_s() func in NameCard class constructor.

I tried debugging, but I could not find the cause.

Could you help me? This is full source code

Have a nice day.

NameCard(char *name, char *phone, char *company, char *position)
{
    this->name = new char(strlen(name) + 1);
    this->phone = new char(strlen(phone) + 1);
    this->company = new char(strlen(company) + 1);
    this->position = new char(strlen(position) + 1);

    strncpy_s(this->name, strlen(name) + 1, name, strlen(name));
    strncpy_s(this->phone, strlen(phone) + 1, phone, strlen(phone));
    strncpy_s(this->company, strlen(company) + 1, company, strlen(company));
    strncpy_s(this->position, strlen(position) + 1, position, strlen(position));
}

"Did not work" in what way? Do you get errors when compiling? If so, copy-paste the errors to the question. — JJJ, Feb 11 '18 at 13:09
Thank you for answer and sorry for my not enough explanations. this code's error occurs run-time, not compile time... — zer0day, Feb 11 '18 at 13:14
**What** error occurs run-time? What happens when you run the program and what did you expect to happen instead? — JJJ, Feb 11 '18 at 13:16
@MichaelWalz But looking at the signature of the class and the OP's details, this is a constructor... Also, `strncpy_s` is a C function that I can't seem to use on a C++ compiler. — Arnav Borborah, Feb 11 '18 at 13:17
@JJJ When i run this program, occurs interrupt message box at strncpy_s() in constructor — zer0day, Feb 11 '18 at 13:21
The only question I _really_ have is why you are not using [`std::string`](http://en.cppreference.com/w/cpp/string/basic_string). Copying literally requires simple assignment for a C++ string. — Arnav Borborah, Feb 11 '18 at 13:23
@ArnavBorborah Very Thanks. The author used strcpy () to copy strings from the book. So I tried strncpy_s () for security, and I got into trouble and asked a question. Thanks to std :: string. My book is very bad :) — zer0day, Feb 11 '18 at 13:29
@user9345804 You should probably pick a book from [this list](https://stackoverflow.com/a/388282/6525260)! — Arnav Borborah, Feb 11 '18 at 13:30
You've made typos: replace all `new char(X)` by `new char[X]`. But the best solution is using `std::string` — Jabberwocky, Feb 11 '18 at 13:36
It was solved thanks to @MichaelWalz. It was my first question because I was born, but I really appreciate your reply(JJJ, Arnav Borborah). Next time, I will write a more detailed questionnaire. :) — zer0day, Feb 11 '18 at 13:41

Jabberwocky · Accepted Answer · 2018-02-22T13:50:59.760

5

You have misused the new operator.

All lines like:

new char(strlen(name) + 1);

should be replaced by:

new char[strlen(name) + 1];

new char(X) allocates a buffer for one single char which will be filled with the character whose ASCII code is X.

new char[X] allocates a buffer for X chars; this is what you want here.

But best would be using std::string in first place.

edited Feb 22 '18 at 13:50

answered Feb 11 '18 at 13:42

Jabberwocky

48,281
17
65
115

`new char(X)` fills the buffer with the **integer value** X (suitably adjusted to fit in a `char`). There is no inherent connection to ASCII or any other character encoding. – Pete Becker Feb 11 '18 at 18:32

score 3 · Answer 2 · answered Feb 11 '18 at 13:42

3

Here's how I would rework your code for C++17.

#include <cstddef>
#include <iostream>
#include <memory>
#include <string>

using std::cout;
using std::endl;
using std::move;
using std::ostream;
using std::string;

class NameCard;
ostream& operator<<(ostream&, NameCard const&);

class NameCard
{
private:
  string _name;
  string _phone;
  string _company;
  string _position;
public:
  NameCard(string name, string phone, string company, string position)
    : _name{move(name)}
    , _phone{move(phone)}
    , _company{move(company)}
    , _position{move(position)}
  {
  }

  NameCard(NameCard const& c)
    : _name{c._name}
    , _phone{c._phone}
    , _company{c._company}
    , _position{c._position}
  {
  }

  ~NameCard()
  {
  }

  void print(ostream& o) const
  {
    o << _name << " " << _phone << " " << _company << " " << _position;
  }
};

ostream& operator<<(ostream& o, NameCard const& c)
{
  c.print(o);
  return o;
}

int main()
{
  auto James = NameCard{"James", "11231123", "la", "manager"};
  cout << James << endl;
  auto James2 = James;
  cout << James2 << endl;
  return EXIT_SUCCESS;
}

answered Feb 11 '18 at 13:42

Eljay

4,648
3
16
27

Very Very Thank you so much Eljay. :) – zer0day Feb 11 '18 at 13:49
@user9345804 • You are very welcome! I hope the rework is helpful & educational as a demonstration/example. :-) C++ is a powerful language, but has many sharp edges. – Eljay Feb 11 '18 at 13:56
@Eljay Example codes look like other languages. :) I will study hard C ++. – zer0day Feb 11 '18 at 13:59
@user9345804 • Arnav Borborah linked to a broad selection of excellent C++ books. – Eljay Feb 11 '18 at 14:05
@Eljay I guess this is just me being picky, but in main, I don't think you should use auto for the name variable's types, since they aren't that long, and using auto might create a lack of clarity. – Arnav Borborah Feb 11 '18 at 15:19
@ArnavBorborah • I've been sold on the "almost always auto" position of my former co-workers Herb Sutter and Scott Meyer. But I understand that not everyone is on board with that convention. – Eljay Feb 11 '18 at 18:29

Why didn't work C++ code(strncpy_s)?

2 Answers2