C++: assigning values to character pointers

Question

I have made a function to copy chars from location pointed by one ptr to location pointed by another.

I am getting segmentation fault in copy function. Can anyone please tell me what is my mistake

#include<iostream>
using namespace std;
int len(const char* a)
{
    int i=0;
    while(true)
    {
        if(*(a+i)=='\0') return i;
        i++;
    }
}
void cpy(char*s1,const char* s2, int n)
{

    for(int i=0;i<n;i++)
    {
        *(s1+i)=*(s2+i);
    }
}

int main()
{
    const char* a = "honey singh";
    char z = 'v';
    char* b = &z;
    int x=len(a);
    cout<<x<<endl;
    cpy(b,a,x);
} 

I want the copy function to copy n chars from location of pointer a to location of pointer b.

I am not concerned with the initial value of b. I just want a function copy(char* b,const char* a,int n) that copies first n characters from a to b How do I do it?

Answer 1

You can do it only when you create the string as a char array, not with a string literal.

char b[] = "hey are you ready";
cpy(b, a);

It is also possible if you dynamically allocate memory for the string with the pointer b.

But while copying, look out for the capacity of the string being copied into. If you copy a larger string into a smaller one, it can also give a segmentation fault.

Answer 2

The problem is that b points to the address of a single character, not an array. So when you copy all of the a string, you're writing outside the bounds of the object that b points to, which produces undefined behavior. You're probably smashing something in the stack, which causes the segmentation fault.

You need to ensure that the destination of the copy is large enough to hold all of the source string.

char *b = new char[len+1];
cpy(b, a, len);

Also, your cpy function should copy n+1 characters so it copies the trailing null character.

Answer 3

Activate all warnings (e.g. with -Wall):

main.cpp:24:15: warning: deprecated conversion from string constant to 'char*'
     char* b = "hey are you ready";
               ^

The easiest solution would be to use an array of characters instead:

char b[] = "hey are you ready"; // more characters than your original one

Concerning copy: you want to check whether source actually contains n characters or stop beforehand:

void cpy(char * dest, const char* source, int n)
{
    for(int i = 0; *source != '\0' && i < n; ++i)
         // copies n characters or until source has been copied completely
        *dest++ = *source++;
    }
    *dest = '\0';
}

Why is this necessary

In your original code, b is a pointer to characters. Now, pointers can point anywhere aka store any memory addresses, even of those memory sections you're not allowed to change. A string literal is stored in the constant data section of your program*. You're not allowed to alter that stuff. But you do that either way and end up with a segmentation fault.

Therefore, you need to ensure that you alter a version that doesn't reside in the read-only section of your program. The array version does exactly that: it initializes a new array, which has the same content as your original literal.

And by the way, your edit has also undefined behavior. Now b points to a single character. If you were to copy more than one character to the location b is addressing, you would write to some memory, but not the one that actually belongs to b. This results in undefined behavior.

The bottom note is: don't trick the compiler with such things.

Using C++, not C

However, there are some other problems with your code. For starters, it's mostly C. The only part where you actually use C++ is cout. If you were to use std::string, you wouldn't have those problems beforehand:

#include <iostream>
#include <string>

int main(){
    const std::string a = "honey singh";
    std::string b = "hey are you ready";

    const int n = 5;

    b = a.substr(0, n);
}

As you can see, your copy is already part of the standard library, namely std::string::substr

Note that there were some other broken things in your original code, such as copy having quadratic runtime instead of linear, copy copying too much into too little memory, etc.

Answer 4

#include<iostream>
#include<cstring>

using namespace std;

int len(const char* a)
{
    int i=0;
    while(true)
    {
        if(*(a+i)=='\0') return i;
        i++;
    }
}

void cpy(char *&s1,const char* s2, int n)
{
  delete [] s1;
  s1=new char[n];
  strncpy(s1,s2,n);
  s1[n]='\0';
}

int main()
{
  const char* a = "honey singh";
  char z = 'v';
  char* b = &z;  
  int x=len(a);
  cout<<x<<endl;

  cpy(b,a,4); //if you want to assign new value to b
  cout<<b<<endl;  //Output will be 'hone'

}