Trash using memcpy

Question

I'm using memcpy to copy a specific number of chars from a char array to a char *. But when I read the char * have always trash in the end.

I'm using libssh2 lib to send commands to my raspberry pi and receive the output.

libssh2_channel_read will return the number of chars of the output int x and the output text will be on the char buffer[32].

Code I'm using:

char buffer[32];
int x = libssh2_channel_read(channel, buffer, sizeof(buffer));
char * output = (char *)malloc(sizeof(char)*x);
memcpy(output, buffer, x-2); // x - 2 because of "\r\n"
libssh2_channel_free(channel);
channel = NULL;
cout << output << endl;

Example of output:

0══²²²²

I only want the 0

That is not the problem but don't cast the return of `malloc` as there is no need and may hide problems, see [here on SO](https://stackoverflow.com/a/605858/8051589) why. — Andre Kampling, Jul 05 '17 at 12:57
@AndreKampling the answer you link says that one does not need to cast, but nothing about the cast being a real problem — 463035818_is_not_an_ai, Jul 05 '17 at 12:58
@tobi303: As I said it is not the problem... But you may hide things wich could bring trouble. — Andre Kampling, Jul 05 '17 at 13:00
@AndreKampling sorry I misread your "That is not" as refering to the comment before... — 463035818_is_not_an_ai, Jul 05 '17 at 13:01
Possible duplicate: [Garbage being printed when using strcpy](https://stackoverflow.com/questions/11161609/garbage-being-printed-when-using-strcpy). It uses `strcpy` instead of `memcpy` but it s the same thing basically. — NathanOliver, Jul 05 '17 at 13:02
@AndreKampling - https://stackoverflow.com/questions/3477741/why-does-c-require-a-cast-for-malloc-but-c-doesnt — Support Ukraine, Jul 05 '17 at 13:07
@NathanOliver No! Because I not working with strings during the copy (where is the question), only in the end. — Samega 7Cattac, Jul 05 '17 at 13:08
"copy ... from a `char` array to a `char*`" -- you're in deep trouble. You cannot copy characters to a pointer. You **must** copy them to **another array of `char`**, typically one that's **pointed to** by a `char*`. A pointer is not an array and an array is not a pointer. — Pete Becker, Jul 05 '17 at 14:32
@PeteBecker I know that. Everyone understood when I said "copy from char array to a char *" I don't mean literaly like x = y. Sorry if my english is not that good xD — Samega 7Cattac, Jul 05 '17 at 14:40
@Samega7Cattac -- your English is fine. My point is that the **difference** between a pointer and an array is extremely important, and beginners often overlook it. There are quite a few posts where (unlike yours) someone just uses a pointer that doesn't point anywhere. — Pete Becker, Jul 05 '17 at 14:45

janm · Accepted Answer · 2017-07-05T13:05:41.333

9

Welcome to C++.

You are copying the values you care about but not the terminating '\0' character. Assuming x is valid (that is: x > 3 and x <= sizeof(buffer)) you can say:

output[x - 2] = '\0';

after the call to memcpy() and you should get what you expect.

However: when you're dealing with communications and buffers like this you need to be careful and check everything.

edited Jul 05 '17 at 13:05

answered Jul 05 '17 at 13:01

janm

17,976
1
43
61

1

Shouldn't that be `output[x-2]` ? – Support Ukraine Jul 05 '17 at 13:04
Correct!! if I do `output[x - 2] = '\0'` will work! Why `x - 2`? Because of the `\r\n` in the end of all commands. – Samega 7Cattac Jul 05 '17 at 13:05
@4386427 Yes, indeed! – janm Jul 05 '17 at 13:05
1

@Samega7Cattac - because a C-style string requires a termination char with the value zero. So if you have `abcdef\r\n` and only copies `abcdef` into the new buffer using `memcpy`, you don't have the termination. So you must add `'\0'` after the `memcpy` to make it a valid C-style string. – Support Ukraine Jul 05 '17 at 13:10

moooeeeep · Answer 2 · 2017-07-05T13:12:25.093

0

I think you shouldn't be using raw arrays and memcpy and the like here.

You are probably better off with the containers from the C++ standard library:

Example:

std::vector<char> buffer(32);
int x = libssh2_channel_read(channel, &buffer[0], buffer.size());
// probably x will be smaller or equal the current size of the buffer
buffer.resize(x);
// if it's a string, why not have it as a std::string
std::string data(buffer.begin(), buffer.end());
std::cout << data << '\n';

edited Jul 05 '17 at 13:12

answered Jul 05 '17 at 13:09

moooeeeep

31,622
22
98
187

This does not remove the `\r\n` at the end of the data – NathanOliver Jul 05 '17 at 13:10
@NathanOliver Resizing a std::string or a std::vector is trivial. Stripping the string of trailing whitespace is left as an exercise for the reader. – moooeeeep Jul 05 '17 at 13:16

score 0 · Answer 3 · answered Jul 05 '17 at 13:17

0

Use std::string:

char buffer[32];
int x = libssh2_channel_read(channel, buffer, sizeof(buffer));
std::string output{ buffer, buffer + x - 2 };
libssh2_channel_free(channel);
channel = NULL;
cout << output << endl;

answered Jul 05 '17 at 13:17

Can you explain why `std::string output{ buffer, buffer + x - 2 };`? – Samega 7Cattac Jul 05 '17 at 13:19
`buffer + x` one after the end of data, `- 2` because of "\r\n" – Jul 05 '17 at 13:24
@Samega7Cattac You give it the (address of) the first byte you _want_ and of the first byte you _don't_ want (one past the end less two for the CRLF). – TripeHound Jul 05 '17 at 13:31
@manni66 Ok, now I understand. But I need it to be char * because of other functions and converting string to char * sometimes can go bad – Samega 7Cattac Jul 05 '17 at 13:32

Trash using memcpy

3 Answers3