Unable to use string as class atribute (see the Update 4)

95 views Asked by At

Take this class as example:

#include <string>
using namespace std;

class One {
private:
  string * text;
public:
  One();
  ~One();

  void setText(string value);

  string uppercase();
  string lowercase();
  string inverted();
};

its implementation:

#include "libone.h"

One::One() {
  text = new string();
}

One::~One() {
  delete text;
}

void One::setText(string value) {
  //
}

string One::uppercase() {
  string result(*this->text);

  for(int i=0; i<result.size(); i++) {
    char c = result[i];
    c = toupper(c);
    result[i] = c;
  }

  return result;
}

string One::lowercase() {
  string result(*this->text);

  for(int i=0; i<result.size(); i++) {
    char c = result[i];
    c = tolower(c);
    result[i] = c;
  }

  return result;
}

string One::inverted() {
  string result;

  string temp(*this->text);
  for(int i=temp.size(); i>0; i--) {
    result = result + temp.at(i);
  }

  return result;
}

used here:

#include <libone.h>

int main(int argc, char *argv[]) {
  One one;
  one.setText("Kleber Mota de Oliveira");
  cout << "text:" << one.uppercase() << endl;
  cout << "text:" << one.lowercase() << endl;
  return 1;
}

programa compile without errors or warnings, but when I try to run the executable, I got an segmentation fault.

I tried use string text as atribute too, but the same problem happens.

Something similar is happening with another project mine. Anyone can tell me what I am doing wrong here?

Makefile for libone.cpp

all: libone

libone: ${obj_dir}/libone.o
    g++ -shared -o ${release_dir}/libone.so ${obj_dir}/libone.o -Wl,--out-implib,${release_dir}/libone.a

${obj_dir}/libone.o: src/libone.cpp
    g++ -fPIC -c src/libone.cpp -o ${obj_dir}/libone.o

Makefile for Main.cpp

all: main

main: ${obj_dir}/Main.o
    g++ -L ${release_dir} -o ${release_dir}/project -Wl,-rpath='./' ${obj_dir}/Main.o -lone -ltwo

${obj_dir}/Main.o: Main.cpp
    g++ -I ${lib_one_header} -I ${lib_two_header} -c Main.cpp -o ${obj_dir}/Main.o

Makefile

export base_dir := ${CURDIR}
export obj_dir := ${base_dir}/build
export release_dir := ${base_dir}/release

export main_src := ${base_dir}/src
export lib_one_header := ${base_dir}/src/libone/include
export lib_one_src := ${base_dir}/src/libone
export lib_two_header := ${base_dir}/src/libtwo/include
export lib_two_src := ${base_dir}/src/libtwo

lib_one := ${lib_one_src}
lib_two := ${lib_two_src}
libraries := $(lib_one) $(lib_two)
player    := ${main_src}

.PHONY: all $(player) $(libraries)
all: $(player)

$(player) $(libraries): | ${obj_dir} ${release_dir}
    $(MAKE) --directory=$@

$(player): $(libraries)

UPDATE 1

If I try build libone.cpp and Main.cpp together with:

g++ libone.cpp Main.cpp -o main

generating only one executable, everything works fine. But if I build with the Makefiles, to get separate library and executable, then I got this Segmentation fault error.

UPDATE 2

result of the backtrace command in gdb shell (using string text as atribute:

#0  0x00007ffff7bd84ec in __memmove_avx_unaligned_erms () from /usr/lib/libc.so.6
#1  0x00007ffff7ea6296 in std::char_traits<char>::copy (__n=23, __s2=<optimized out>, __s1=<optimized out>)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:409
#2  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy (__n=23, __s=<optimized out>, __d=<optimized out>)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:359
#3  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_S_copy (__n=23, __s=<optimized out>, __d=<optimized out>)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:354
#4  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_assign (this=this@entry=0x7fffffffd9ce, __str="Kleber Mota de Oliveira")
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:272
#5  0x00007ffff7ea662f in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign (__str=..., this=0x7fffffffd9ce)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:1355
#6  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator= (this=0x7fffffffd9ce, __str=...)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:680
#7  0x00007ffff7fba255 in One::setText(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) () from ./libone.so
#8  0x000055555555637d in main ()

(using string * text as atribute):

#0  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_assign (this=this@entry=0x55555556bee0beb0, __str="Kleber Mota de Oliveira")
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:260
#1  0x00007ffff7ea662f in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign (__str=..., this=0x55555556bee0beb0)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:1355
#2  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator= (this=0x55555556bee0beb0, __str=...)
    at /usr/src/debug/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:680
#3  0x00007ffff7fba2a6 in One::setText(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) () from ./libone.so
#4  0x000055555555637d in main ()

UPDATE 3

With this update, only the first method in Main.cpp is executed. In the second I got a Segmentation fault error:

class One {
private:
  char * text;
public:
  One();
  ~One();

  void setText(char* value);

  string uppercase();
  string lowercase();
  string inverted();
};

implementation:

void One::setText(char* value) {
  text = value;
}

string One::uppercase() {
  string result = text;

  for(int i=0; i<result.size(); i++) {
    char c = result[i];
    c = toupper(c);
    result[i] = c;
  }

  return result;
}

string One::lowercase() {
  string result(text);

  for(int i=0; i<result.size(); i++) {
    char c = result[i];
    c = tolower(c);
    result[i] = c;
  }

  return result;
}

string One::inverted() {
  string result;

  string temp(text);
  for(int i=temp.size()-1; i>=0; i--) {
    result = result + temp[i];
  }

  return result;
}

UPDATE 4

with the code below, all methods called in Main.cpp are executed, but display a bunch of rubish in the screen. An example:

text:�Q���
text:�Q���
text:���t

class declaration:

class One {
private:
  char * text;
public:
  One();
  ~One();

  void setText(char* value);

  const char * uppercase();
  const char * lowercase();
  const char * inverted();
};

implementation:

const char * One::uppercase() {
  string result = text;

  for(int i=0; i<result.size(); i++) {
    char c = result[i];
    c = toupper(c);
    result[i] = c;
  }

  return result.c_str();
}

const char * One::lowercase() {
  string result(text);

  for(int i=0; i<result.size(); i++) {
    char c = result[i];
    c = tolower(c);
    result[i] = c;
  }

  return result.c_str();
}

const char * One::inverted() {
  string result;

  string temp(text);
  for(int i=temp.size()-1; i>=0; i--) {
    result = result + temp[i];
  }

  return result.c_str();
}

UPDATE 5

class declaration:

class One {
private:
  char * text;
public:
  One();
  ~One();

  void setText(const char * value);

  const char * uppercase();
  const char * lowercase();
  const char * inverted();
};

implementation:

#include "libone.h"

#include <iostream>
using namespace std;

One::One() {
  text = new char[1];
  text[0] = '\0';
}

One::~One() {
  delete [] text;
}

void One::setText(const char * value) {
  delete [] text;

  int size = 0;
  for(; value[size] != '\0'; size++);

  text = new char[size];
  for(int i=0; i<size; i++)
    text[i] = value[i];
}

const char * One::uppercase() {
  char * result;

  int size = 0;
  for(; text[size] != '\0'; size++);

  result = new char[size];
  for(int i=0; i<size; i++) {
    if (text[i] >= 65 && text[i] <= 90)
      result[i] = text[i] - 32;
    else
      result[i] = text[i];
  }

  return result;
}

const char * One::lowercase() {
  char * result;

  int size = 0;
  for(; text[size] != '\0'; size++);

  result = new char[size];
  for(int i=0; i<size; i++) {
    if (text[i] >= 33 && text[i] <= 58)
      result[i] = text[i] + 32;
    else
      result[i] = text[i];
  }

  return result;
}

const char * One::inverted() {
  char * result;

  int size = 0;
  for(; text[size] != '\0'; size++)

  result = new char[size];
  for(int i=0; i<size; i++) {
    result[i] = text[size-(i+1)];
  }
  result[size] = '\0';

  return result;
}

Main.cpp

#include "libone.h"

#include <iostream>
using namespace std;

int main(int argc, char *argv[]) {
  One one;
  one.setText(argv[1]);

  cout << one.uppercase() << endl;
  cout << one.lowercase() << endl;
  cout << one.inverted() << endl;

  return 1;
}

output:

+leber -ota de /liveira
Kleber Mota de Oliveira
arievilO ed atoM rebelK
*** stack smashing detected ***: terminated
zsh: IOT instruction (core dumped)
1

There are 1 answers

11
Remy Lebeau On

There is no good reason to make the text member be a string* pointer (especially since you are not following the Rule of 3/5/0 to manage the pointer properly, but we can ignore that for this exercise since you are not making any copies of the one object).

But, let's just say, for the sake of argument, that you need a pointer, for whatever reason. The only mistakes your code is making are:

  • in setText(), you are not assigning value to text, eg:
void One::setText(string value) {
  *this->text = value;
}
  • in inverse(), you are accessing the temp string out of bounds, it should be like this instead:
string One::inverted() {
  string result;

  string temp(*this->text);
  for(int i=temp.size()-1; i>=0; i--) {
    result = result + temp[i];
  }

  return result;
}

The rest of the code you have shown works fine:

Online Demo

UPDATE:

The problem with your code is that you are implementing One in a dynamic library. As such, you can't pass non-POD types, like std::string, across the library boundary. So, try something more like this instead:

class One {
private:
  char * text;
public:
  One();
  ~One();

  void setText(const char *value);

  char* uppercase();
  char* lowercase();
  char* inverted();
};

void freeString(char *str);
#include "libone.h"
#include <cstring>
#include <cctype>
using namespace std;

One::One() {
  text = new char[1];
  *text = '\0';
}

One::~One() {
  delete[] text;
}

void One::setText(const char *value) {
  delete[] text;
  text = new char[strlen(value)+1];
  strcpy(text, value);
}

char* One::uppercase() {
  int len = strlen(text);

  char* result = new char[len+1];
  strcpy(result, text);

  for(int i = 0; i < len; i++) {
    unsigned char c = result[i];
    c = toupper(c);
    result[i] = c;
  }

  return result;
}

char* One::lowercase() {
  int len = strlen(text);

  char *result = new char[len+1];
  strcpy(result, text);

  for(int i = 0; i < len; i++) {
    unsigned char c = result[i];
    c = tolower(c);
    result[i] = c;
  }

  return result;
}

char* One::inverted() {
  int len = strlen(text);

  char *result = new char[len+1];

  for(int i = 0; i < len; i++) {
    result[i] = text[len-1-i];
  }

  return result;
}

void freeString(char *str) {
    delete[] str;
}
#include <libone.h>

int main(int argc, char *argv[]) {
  One one;
  one.setText("Kleber Mota de Oliveira");

  char *str = one.uppercase();
  cout << "text:" << str << endl;
  freeString(str);

  str = one.lowercase();
  cout << "text:" << str << endl;
  freeString(str);

  str = one.inverted();
  cout << "text:" << str << endl;
  freeString(str);

  return 1;
}

Alternatively:

class One {
private:
  void * text;
public:
  One();
  ~One();

  void setText(const char *value);

  char* uppercase();
  char* lowercase();
  char* inverted();
};

void freeString(char *str);
#include "libone.h"
#include <string>
using namespace std;

One::One() {
  text = new std::string;
}

One::~One() {
  delete static_cast<std::string*>(text);
}

void One::setText(const char *value) {
  *static_cast<std::string*>(text) = value;
}

char* One::uppercase() {
  std::string temp = *static_cast<std::string*>(text);

  for(size_t i = 0; i < temp.size(); i++) {
    unsigned char c = temp[i];
    c = toupper(c);
    temp[i] = c;
  }

  char* result = new char[temp.size()+1];
  strcpy(result, temp.c_str());
  return result;
}

char* One::lowercase() {
  std::string temp = *static_cast<std::string*>(text);

  for(size_t i = 0; i < temp.size(); i++) {
    unsigned char c = temp[i];
    c = tolower(c);
    temp[i] = c;
  }

  char *result = new char[temp.size()+1];
  strcpy(result, temp.c_str());
  return result;
}

char* One::inverted() {
  std::string &str = *static_cast<std::string*>(text);

  std::string temp;
  temp.reserve(str.size());

  for(size_t i = str.size(); i > 0; i--) {
    temp += str[i-1];
  }

  char *result = new char[temp.size()+1];
  strcpy(result, temp.c_str());
  return result;
}

void freeString(char *str) {
    delete[] str;
}
#include <libone.h>

int main(int argc, char *argv[]) {
  One one;
  one.setText("Kleber Mota de Oliveira");

  char *str = one.uppercase();
  cout << "text:" << str << endl;
  freeString(str);

  str = one.lowercase();
  cout << "text:" << str << endl;
  freeString(str);

  str = one.inverted();
  cout << "text:" << str << endl;
  freeString(str);

  return 1;
}