1

I've tried to change the code to do what I want in different ways but doesn't seem to fix that Valgrind error.

Cadena::Cadena(unsigned tam, char c):s_(new char[tam+1]), tam_(tam)
{
    //c is ' ' by default
    memset(s_, c, tam+1);
    s_[tam]='\0';
}
...
Cadena& Cadena::substr(unsigned i, unsigned tam) const
{
    if(i>=tam_||i+tam>tam_) throw out_of_range("Posicion indicada fuera de rango");
    Cadena* subCad=new Cadena(tam);
    strncpy(subCad->s_, &s_[i], tam);
    return *subCad;
}

==9669== Invalid write of size 1
==9669==    at 0x1390CC: Cadena::Cadena(unsigned int, char) (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==    by 0x13972B: Cadena::substr(unsigned int, unsigned int) const (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==    by 0x12D436: test_cadena(_fctkern_t*) (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==    by 0x137D42: main (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==  Address 0x105bc176f is not stack'd, malloc'd or (recently) free'd
==9669== 
==9669== 
==9669== Process terminating with default action of signal 11 (SIGSEGV)
==9669==  Access not within mapped region at address 0x105BC176F
==9669==    at 0x1390CC: Cadena::Cadena(unsigned int, char) (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==    by 0x13972B: Cadena::substr(unsigned int, unsigned int) const (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==    by 0x12D436: test_cadena(_fctkern_t*) (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==    by 0x137D42: main (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==9669==  If you believe this happened as a result of a stack
==9669==  overflow in your program's main thread (unlikely but
==9669==  possible), you can try to increase the size of the
==9669==  main thread stack using the --main-stacksize= flag.
==9669==  The main thread stack size used in this run was 8388608.

Another way:

Cadena& Cadena::substr(unsigned i, unsigned tam) const
{
    if(i>=tam_||i+tam>tam_) throw out_of_range("Posicion indicada fuera de rango");
    char res[tam+1];
    strncpy(res, s_+i, tam);
    res[tam]='\0';
    Cadena* subCad=new Cadena(tam);
    return *subCad;
}

==17029== Invalid write of size 1
==17029==    at 0x4C3304C: strncpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17029==    by 0x13979E: Cadena::substr(unsigned int, unsigned int) const (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==17029==  Address 0x1fff001000 is not stack'd, malloc'd or (recently) free'd
==17029== 
==17029== 
==17029== Process terminating with default action of signal 11 (SIGSEGV)
==17029==  Access not within mapped region at address 0x1FFF001000
==17029==    at 0x4C3304C: strncpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17029==    by 0x13979E: Cadena::substr(unsigned int, unsigned int) const (in /mnt/hgfs/mvpoosf/P1/test-P1-auto)
==17029==  If you believe this happened as a result of a stack
==17029==  overflow in your program's main thread (unlikely but
==17029==  possible), you can try to increase the size of the
==17029==  main thread stack using the --main-stacksize= flag.
==17029==  The main thread stack size used in this run was 8388608.

I'm passing this test (test-P1-auto):

  FCT_TEST_BGN(Cadena - Subcadena: tamanno mayor que longitud restante) {
    const Cadena a("0123456789");
    fct_chk_ex(out_of_range&, a.substr(9, 2));
  }
  FCT_TEST_END();

It returns a substring of two elements following the number 8. It's not possible as it throws out_of_range, so the test is OK. But when I use Valgrind, it's giving me those errors and I've been stuck here for hours.

Private part of my class Cadena:

private:
    char* s_;
    unsigned int tam_;
Francisco Rosa
  • 11
  • 1
  • 2
    Can you show a [mre] that anyone can compile ***exactly as shown*** and reproduce your valgrind diagnostic? – Sam Varshavchik Aug 27 '20 at 15:50
  • 3
    Do you build with debug information? Then Valgrind should be able to show exact line number where the problem is. – Some programmer dude Aug 27 '20 at 15:51
  • 1
    Unrelated to the error you're seeing, but returning a reference to a `new`ed object is absolutely terrible practice that's _very_ likely to lead to memory leaks. – Miles Budnek Aug 27 '20 at 15:54
  • Avoid bare owning pointers. Use `std::string` instead. – eerorika Aug 27 '20 at 16:13
  • That's not the constructor used in your test. Please post the constructor that can take a string literal. – molbdnilo Aug 27 '20 at 17:24
  • Note that `char res[tam+1];` defines `res` as a [*variable-length array*](https://en.wikipedia.org/wiki/Variable-length_array), and they are not part of standard C++. Unless this is an exercise in using pointers and dynamic allocation, you should (as mentioned already) use `std::string` for all strings. – Some programmer dude Aug 27 '20 at 18:36
  • Also `return *subCad;` after `Cadena* subCad=new Cadena(tam);` leads to a memory leak (you loose the original object, you can't `delete` it later). And what's worse, unless you follow [the rules of three, five or zero](https://en.cppreference.com/w/cpp/language/rule_of_three) you probably have the reason for your problem there. – Some programmer dude Aug 27 '20 at 18:38

1 Answers1

0

Sorry for the mistake guys, but as @Some programmer dude commented I didn't put the flag -g on my makefile that allows valgrind to tell me wich lines make the errors. Actually that wasn't the right test, it was the next one:

FCT_TEST_BGN(Cadena - Subcadena: tamanno negativo) {
    const Cadena a("0123456789");
    fct_chk_ex(out_of_range&, a.substr(9, -1));
  }
  FCT_TEST_END();

As I can see here Signed to unsigned conversion in C - is it always safe?, there's no way to get a negative value as unsigned and expect the sign "just to be removed". I solved it by changing it to int and checking inside the function if it's negative.

Thanks everyone for your help.

Francisco Rosa
  • 11
  • 1