C++ char* problems

Question

#include<iostream>
using namespace std;
int main()
{
    char *name="Siva",*str;
    for(int i=0;i<strlen(name);i++)
    {
        str[i]=name[i];
    }
    cout<<str;
    return 0;
}

The first program gives output Sivaœ> i.e siva with some garbage values.... But the second program show segmentation fault...Please help me to find out exact answer...

#include<iostream>
using namespace std;
int main()
{
    int i=0;
    char *name="Siva",*str;
    for(i=0;i<strlen(name);i++)
    {
        str[i]=name[i];
    }
    cout<<str;
    return 0;
}

What would you like an answer **too**? You have only described how your programs behave, not what the problem is, or what you *want* them to do, or what you *expect* them to do — jalf, Jul 12 '13 at 13:16
`char *name="Siva"` shouldn't even compile without error (or at least warning). — Nawaz, Jul 12 '13 at 13:21
I am just trying to copy the value from name to str..But in first case it works properly...Why it give correct answer for first case...Please tell me that first — user2390140, Jul 12 '13 at 13:21
Not too sure why the first program doesn't give a segmentation fault either... — Mash, Jul 12 '13 at 13:21
You should accept an answer to this question. Click the checkmark next to the answer that you found most helpful for you (if helpful at all). — David G, Jul 12 '13 at 16:26

score 5 · Answer 1 · answered Jul 12 '13 at 13:21

char *name="Siva",*str;
for(int i=0;i<strlen(name);i++)
{
str[i]=name[i];
}

str is a pointer, but it doesn't yet point to anything.

Since you're in C++, you should be using std::string:

#include<iostream>
#include <string>

using namespace std;
int main()
{
  char *name="Siva";
  std::string str;
  for(int i=0;i<strlen(name);i++)
  {
    str += name[i];
  }
  cout<<str;
  return 0;
}

Even better, get rid of the hand-written loop:

#include <algorithm>
int main()
{
  char *name="Siva";
  std::string str;
  std::copy (name, name + strlen (name), std::back_inserter (str));
  cout<<str;
  return 0;
}

Better even still, there's no reason in this particular example why you need to do any of that at all:

char* name = "Silva";
std::string str = name;
cout << str;

By the way, string literals in C++ are inherently const:

const char* name = "Silva";

If you really must use a char*, first I would strongly question why, and then I would tell you to do this:

int main()
{
  const char *name="Siva";
  char* str = new char [strlen (name)+1]; // +1 for the null terminator
  strcpy (name, str);
  cout << str;
  delete [] str;
  return 0;
}

I would even more strongly question your need to copy it byte-by-byte:

int main()
{
  const char *name="Siva";
  char* str = new char [strlen (name)+1]; // +1 for the null terminator
  for (size_t i = 0; i < strlen (name); ++i )
    str [i] = name [i];
  str [strlen (name)] = '\0';
  cout << str;
  delete [] str;
  return 0;
}

I'll have to assume you're playing the pedant professor here: `std::copy (name, name + strlen (name), std::back_inserter (str))`. Come on — sehe, Jul 12 '13 at 13:31
@sehe: Yeah, just trying to show a few different ways to skin the cat. — John Dibling, Jul 12 '13 at 13:34

score 3 · Answer 2 · answered Jul 12 '13 at 13:15

3

You have undefined behaviour here:

str[i]=name[i];

str has not been initialized to anything. You are writing to places you shouldn't.

answered Jul 12 '13 at 13:15

juanchopanza

223,364
34
402
480

score 3 · Answer 3 · answered Jul 12 '13 at 13:18

There are two problems with this.

The pointer str doesn't point to allocated memory, so writing through it is undefined behavior.
Even if it did point to valid memory, you're not writing the correct amount of data. When copying a string, you need to copy the 0 byte at the end which marks the end of the string; so the upper limit of your loop should bt strlen(name) + 1. Or you could use a library method like strdup() instead of your own for loop.

The reason the "working" version prints some garbage characters is that there's no 0 at the end of the copied string to tell iostreams to stop printing. The reason the "working" one doesn't crash, and the other one does, is pure dumb luck: the garbage in str, by chance, points to memory you're allowed to write to, while in the crashing program, it points to memory you're not allowed to write to. Simple as that.

Pierre Fourgeaud · Answer 4 · 2013-07-12T13:36:49.977

2

It is because you have no memory allocated for str. (it will cause an undefined behavior)

You can mix that by using a merory allocation function like in this example :

#include<iostream>
using namespace std;
int main()
{
    char *name="Siva",*str;
    // Allocate memory with malloc
    str = (char*)malloc( (strlen(name)+1) * sizeof(char) );
    for(int i=0;i<strlen(name);i++)
    {
        str[i]=name[i];
    }
    str[strlen(name)] = 0;
    cout<<str;
    // Free the allocated memory
    free(str);
    return 0;
}

As you are using c++, you can do :

#include<iostream>
using namespace std;
int main()
{
    char *name="Siva",*str;
    // Allocate memory with new
    str = new char[strlen(name) + 1];
    for(int i=0;i<strlen(name);i++)
    {
        str[i]=name[i];
    }
    str[strlen(name)] = 0;
    cout<<str;
    // Free the allocated memory
    delete []str;
    return 0;
}

EDIT : The reason you have some weird caracters at the end of your ouput is that because you string is not terminated with a '\0', it will continue to print it. (This will occur only if you don't have a segmentation fault )

edited Jul 12 '13 at 13:36

answered Jul 12 '13 at 13:16

Pierre Fourgeaud

14,290
1
38
62

Then how it gives that answer – user2390140 Jul 12 '13 at 13:18
1

It is an undefined behaviour. Sometimes, the operating system will let your program access to that memory. But it is more common to end up with a `segmentation fault` ... – Pierre Fourgeaud Jul 12 '13 at 13:20
"You should do" - This must be some strange meaning of the word "should" that I was previously unaware of. – sehe Jul 12 '13 at 13:28
I changed it, you were right. – Pierre Fourgeaud Jul 12 '13 at 13:37

score 1 · Answer 5 · edited May 23 '17 at 10:32

1

str[i]=name[i]; is illegal, causes Undefined behavior, because you have not allocated memory for str.

Before for loop allocate memory for destination string str:

str = malloc (strlen(name) + 1 );

Also you forgetting string termination, after for-loop add terminate string str[i] = '\0';
Undefined behavior refers to computer code whose behavior is unpredictable.

You code should be:

char *name="Siva", *str;
str = malloc (strlen(name) + 1 ); // mistake 
for(int i=0;i<strlen(name);i++)
{
    str[i]=name[i];
}
str[i] = '\0';  // forgetting

To understand further you can read this answer: strcat() implementation works but causes a core dump at the end

edited May 23 '17 at 10:32

Community

1
1

answered Jul 12 '13 at 13:15

Grijesh Chauhan

57,103
20
141
208

Then how it gives answer for first case...... – user2390140 Jul 12 '13 at 13:15
It's undefined behavior. Anything can happen. – tangrs Jul 12 '13 at 13:16
2

"Undefined" means "It might work, and it might not." – Ernest Friedman-Hill Jul 12 '13 at 13:16
2

Actually, I think "it might work" is misleading. It might _appear_ to work. But _never_ actually (provably/reliably) – sehe Jul 12 '13 at 13:25
@sehe yes its worst case when program executes correctly even under undefined behavior – Grijesh Chauhan Jul 12 '13 at 13:43
.... ugh. "executes correctly". No it doesn't. It might provide the visible output you expect, but that's only because humans don't look very closely. – sehe Jul 12 '13 at 13:48

score 1 · Answer 6 · answered Jul 12 '13 at 13:20

There are a couple problems with your code.

Firstly, *str is not allocated, so it starts off by pointing to whatever bit of memory the pointer value happens to start off as.

Secondly, strlen() returns the length of the string excluding the terminating null character. So what you are doing is copying all the values of name into some random bit of memory, not terminating it, then telling the system to print that off, which could be any length.

score 0 · Answer 7 · answered Jul 12 '13 at 13:23

Your problem is using char arrays and pointers to represent strings in language which has proper string type.

#include <iostream>
#include <string>
using namespace std;

int main()
{
    string name = "Siva", str;
    str = name;
    cout << str;
    return 0;
}

score 0 · Answer 8 · answered Jul 12 '13 at 13:40

0

Problem is with str[i]=name[i] you must know that C++ does not care for memory leaks as Java or some others. So you must allocate memory to a variable or pointer in order to avoid these problems. Already there are number of answers, you can also try

str=new char[strlen(name)+1];

and do not forget to terminate the char array with null when you done copy. In this case

str[strlen(name)]='\0';

answered Jul 12 '13 at 13:40

Shahrzad

1,062
8
26

You mention memory leaks, yet fail to mention `delete` which has to be called to avoid memory leaks :/ Also, if you're coming from Java, you may be surprised with the fact that C++ has a string type (`std::string`), which manages memory (RAII). – milleniumbug Jul 12 '13 at 13:48
That is not the case. I know C++ more than Java. May be you are surprised when I was a student I was using Turbo C++, no String class, then I made a full string handling there by my self. – Shahrzad Jul 12 '13 at 13:57
I know, and *I don't care* - I checked my calendar and saw it's now year 2013. Nevertheless, my point was you don't mention `delete[]` has to be used after `new[]`. Also, as you see, C++ evolves (`std::string` was added in *1998*), as well as its usage. – milleniumbug Jul 12 '13 at 14:04

C++ char* problems

8 Answers8