Dereferencing char* pointer with an offset giving me segmentation faults.

[mothergoose729]

I have a class in which a char* is a member of that class. I have initialized the members to "abcde". I am trying to use a member function that changes the casing of the letters in the class; for example "abcde" would be changed to "ABCDE".

my main

#include <iostream>
#include "MyString.h"

using namespace std;

int main()
{
    STRING s;
    STRING s1 = "abcde";
    STRING s2 = "c";
    cout << s1.length() << endl;
    cout << s1.position('c') << endl;
    cout << s1 << endl;
    s1.upcase();
    cout << s1 << endl;
    cin.get();
    return 0;
}

My header file

#ifndef _MYSTRING_H_ 
      #define _MYSTRING_H_ 

      #include <iostream>
      using namespace std;

      class STRING
      {
      private:
              
        char* _s; 
        unsigned _len; 
        
      public: 
        STRING(); //default constructor
        STRING(char*);//constructor
        STRING(char); //constructor
        STRING(const STRING&);//copy constructor
        ~STRING(); //destructor
        
        int length() const;
        int position(char) const;
        void upcase(void);
        friend ostream& operator<< ( ostream&, const STRING&);

My implementation file

#include "MyString.h"


STRING::STRING()
{
    _s = NULL;
    _len = 0;
}

STRING::STRING(char* c)
{
    int i = 0;
    _s = c;
    
    while ( c[i] != '\0')
    {
        i++;
    }
    
    _len = i;
}

STRING::STRING(char c)
{
    _s = &c;
    _len = 1;
}

STRING::STRING(const STRING& s)
{
    _s = s._s;
    _len = s._len;
}

STRING::~STRING()
{
    delete[] _s;
}
       
int STRING::length() const
{
    return _len;
}

int STRING::position(char inst) const
{
     for (int i = 0; i < _len; i++)
    {
        if (inst == _s[i])
        {
                 return i;
        }
    }
    
    return -1;
}
void STRING::upcase(void)
{
    for(int i = 0; i < _len; i++)
    {
        if (_s[i] >= 'a' && _s[i] <= 'z')
        {
            _s[i] &= 223; 
        }
    }
}

This last function is the one giving me trouble

void STRING::upcase(void)
{
    for(int i = 0; i < _len; i++)
    {
        if (_s[i] >= 'a' && _s[i] <= 'z')
        {
            _s[i] &= 223; 
        }
    }
}

Whenever I try an access the elements pointed to by _s the program hangs and crashes, presumably from a segmentation fault. Is this not legal? I don't understand how I am indexing bad data.

[BlackSheep]

You're constructing the string using a string literal, which is of type const char*, and can't be modified.

EDIT: Also note that this is a pretty dangerous string class. Since each STRING only points to the string it represents and doesn't actually have a local copy, if one STRING changes a shared string value, it'll effect all of the STRINGs pointing to that one.

[mothergoose729]

Thank you blacksheep, I didn't know that. What would be the appropriate alternative for implementing this class?

I attempted to re implement my class using a character array, and my program is still crashing.

header file

#ifndef _MYSTRING_H_ 
      #define _MYSTRING_H_ 

      #include <iostream>
      using namespace std;

      class STRING
      {
      private:
              
        char _s[0]; 
        unsigned _len; 
        
      public: 
        STRING(); //default constructor
        ~STRING(); //destructor
       friend ostream& operator<< ( ostream&, const STRING&);
}

#endif 

Implementation

#include "MyString.h"


STRING::STRING()
{
    _s[0] = '\0';
    _len = 0;
}

STRING::~STRING()
{
    delete[] _s;
}

ostream& operator<< ( ostream& os, const STRING& s)
{
    for(int i = 0; i < s._len; i++)
    {
        os << s._s[i];
    }
} 

main

#include <iostream>
#include "MyString.h"

using namespace std;

int main()
{
    STRING s;
    cout << s << endl;
    cin.get();
    return 0;
}

I believe the problem is in my ostream friend function, but I don't know what it is.

[vlad from moscow]

You could escape the error if you would correctly define the constructor. Instead of

STRING(char*);//constructor

It would be better to define it as

STRING( const char* );//constructor

[mothergoose729]

I am confused vlad. Are you saying that if I changed my STRING(char*) constructor that I could use my old implementation? Would it be possible to access and change members pointed to by char* by declaring it const?

[zb0S216C]

You're not allowed to dereference a char * that points to a string literal, because string literals are ROM (outside an array declaration). Writing to a string literal is not noticed during compilation; thus, the system will throw a run-time exception to indicate a violation of the ROM (Read-Only Memory). For example, here's what's allowed, and what isn't, and why:

char *String("String");
String[0] = '...';

This code is unsafe, because "String" is a pointer to non-constant data, but the string literal it points to is read-only. Attempting to write to this ROM is detected during run-time. So when the time comes to dereference "String", the system will realise you're attempting to alter data it marked as ROM, and will throw an exception; cue the seg-fault.

const char *String("String");

This code is safe, because "String" is a pointer to ROM, which corresponds the const-ness of a string literal. Attempting to dereference "String" will result in a compile-time error, so there's no chance of the system throwing a run-time exception.

char Array[] = "String";
Array[0] = '...';

This code is also safe, because you're not assigning "Array" to a string literal, although, it does appear as though you are. As a result, writing to ""Array"" is a safe operation, which both compiler and system respect.

Based on what I said above, your "STRING" class is unsafe, and prone to run-time errors. Consider std::string, or dynamically allocate the array.

_Wazzak

[mothergoose729]

I figured out my problem. I re implemented my upcase member function to work as follows:

void STRING::upcase(void)
{
    char* temp = new char[_len-1];
     
    for(int i = 0; i < _len; i++)
    {
        temp[i] = _s[i];
        if (_s[i] >= 'a' && _s[i] <= 'z')
        {
            temp[i] &= 223;; 
        }
    }
    
     delete[]  _s;
    _s = temp;
}

It is not the most elegant of solutions, but it works. Because the program must use dynamic memory allocation for other member functions I don't think I can use a fixed character array as a member. I would appreciate any feedback on this approach though and suggestions for better solutions.

[vlad from moscow]

If you would change the constructor such a way as I showed then you could not do the stuff as you are doing it . The compiler would not allow to assign const char * to char *.

[mothergoose729]

I see. So I need to re implement my STRING(char*) member function as a STRING(const char*). What would the implementation of this function look like?

[zb0S216C]

You could Implement a string class that encapsulates a character array, and provides functionality for that array (this is how "std::array" is implemented). For example:

template <unsigned int Length>
class String
{
    public:
        String(const char *InitString)
        {
            unsigned char *Position(this->Array);
            while(*InitString)
            {
                *Position = *InitString;
                ++InitString, ++Position;
            }
        }

        String(String const &StringCopy)
        {
            unsigned char *Source(StringCopy.Array);
            unsigned char *Destination(this->Array);
            while(*Source)
            {
                *Destination = *Source;
                ++Source, ++Destination;
            }
        }

    private:
        unsigned char Array[((Length > 0u) ? (Length + 1) : 1u)];

    public:
        unsigned int ObtainLength() const
        {
            return(Length);
        }

        unsigned char &operator [] (unsigned int const Index)
        {
            if(Index >= Length) throw("an exception");
            return(this->Array[index]);
        }

        unsigned char const &operator [] (unsigned int const Index) const
        {
            if(Index >= Length) throw("an exception");
            return(this->Array[index]);
        }
};

Or, better yet, use "std::array" if you have access to C++11.

_Wazzak

[mothergoose729]

So you are saying that I should create another class where its member is a character array, provide functionality for that class, and then build a STRING class that uses this array class as a member? If I am not understanding at all please educate me.

This is a class project I am doing. I will ask my professor about it, but I likely can't do this because it doesn't meet the specifications he set for this assignment. If there is a way I can implement all the safety and functionality without implementing another class, then this is I think the best way for me to complete this project.