Invalid free() / delete / delete[] / realloc() error in assignment operator

Question

I am new to programming and when I am trying to run a program using Valgrind I was getting an error like this. I googled hours to solve this problem. please can you tell me where I am going wrong. Hope there is some mistake near assignment operator.

Error:

==5130== Invalid read of size 8
==5130==    at 0x400CFD: std::passwd::~passwd() (passwd.c++:18)
==5130==    by 0x400C06: main (p1.c++:21)
==5130==  Address 0x5a1c040 is 0 bytes inside a block of size 8 free'd
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D16: std::passwd::~passwd() (passwd.c++:19)
==5130==    by 0x400BFA: main (p1.c++:17)
==5130== 
==5130== Invalid free() / delete / delete[] / realloc()
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D07: std::passwd::~passwd() (passwd.c++:18)
==5130==    by 0x400C06: main (p1.c++:21)
==5130==  Address 0x5a1c420 is 0 bytes inside a block of size 8 free'd
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D07: std::passwd::~passwd() (passwd.c++:18)
==5130==    by 0x400BFA: main (p1.c++:17)
==5130== 
==5130== Invalid free() / delete / delete[] / realloc()
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D16: std::passwd::~passwd() (passwd.c++:19)
==5130==    by 0x400C06: main (p1.c++:21)
==5130==  Address 0x5a1c040 is 0 bytes inside a block of size 8 free'd
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D16: std::passwd::~passwd() (passwd.c++:19)
==5130==    by 0x400BFA: main (p1.c++:17)
==5130== 
==5130== 
==5130== HEAP SUMMARY:
==5130==     in use at exit: 120 bytes in 1 blocks
==5130==   total heap usage: 7 allocs, 8 frees, 729 bytes allocated
==5130== 
==5130== Searching for pointers to 1 not-freed blocks
==5130== Checked 192,896 bytes
==5130== 
==5130== 120 bytes in 1 blocks are still reachable in loss record 1 of 1
==5130==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x53C0024: getdelim (iogetdelim.c:66)
==5130==    by 0x5444B52: getpass (getpass.c:97)
==5130==    by 0x400F84: std::passwd::get() (passwd.c++:66)
==5130==    by 0x400B3C: main (p1.c++:9)
==5130== 
==5130== LEAK SUMMARY:
==5130==    definitely lost: 0 bytes in 0 blocks
==5130==    indirectly lost: 0 bytes in 0 blocks
==5130==      possibly lost: 0 bytes in 0 blocks
==5130==    still reachable: 120 bytes in 1 blocks
==5130==         suppressed: 0 bytes in 0 blocks
==5130== 
==5130== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
==5130== 
==5130== 1 errors in context 1 of 3:
==5130== Invalid free() / delete / delete[] / realloc()
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D16: std::passwd::~passwd() (passwd.c++:19)
==5130==    by 0x400C06: main (p1.c++:21)
==5130==  Address 0x5a1c040 is 0 bytes inside a block of size 8 free'd
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D16: std::passwd::~passwd() (passwd.c++:19)
==5130==    by 0x400BFA: main (p1.c++:17)
==5130== 
==5130== 
==5130== 1 errors in context 2 of 3:
==5130== Invalid free() / delete / delete[] / realloc()
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D07: std::passwd::~passwd() (passwd.c++:18)
==5130==    by 0x400C06: main (p1.c++:21)
==5130==  Address 0x5a1c420 is 0 bytes inside a block of size 8 free'd
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D07: std::passwd::~passwd() (passwd.c++:18)
==5130==    by 0x400BFA: main (p1.c++:17)
==5130== 
==5130== 
==5130== 1 errors in context 3 of 3:
==5130== Invalid read of size 8
==5130==    at 0x400CFD: std::passwd::~passwd() (passwd.c++:18)
==5130==    by 0x400C06: main (p1.c++:21)
==5130==  Address 0x5a1c040 is 0 bytes inside a block of size 8 free'd
==5130==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5130==    by 0x400D16: std::passwd::~passwd() (passwd.c++:19)
==5130==    by 0x400BFA: main (p1.c++:17)
==5130== 
==5130== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

Here is the header file passwd.h

#include<stdio.h>
#include<iostream>
#ifndef MIN_PASSWD_LEN
#define MIN_PASSWD_LEN 6
#endif
#ifndef MAX_PASSWD_LEN
#define MAX_PASSWD_LEN 20
#endif
#ifndef NO_OF_NUMBERS
#define NO_OF_NUMBERS 1
#endif
#ifndef NO_OF_ALPHABETS
#define NO_OF_ALPHABETS 1
#endif
#ifndef NO_OF_SYMBOLS
#define NO_OF_SYMBOLS 1
#endif
namespace std{
  class passwd{
   char **pwd;
  public:
   passwd& operator=(const passwd);
   passwd();
   ~passwd();
   bool check_validity();
   void tochar(char **);
   void get();
 };
}

functions for above header file: passwd.c++

#include<iostream>
#include"passwd.h"
#include<string.h>
#include<unistd.h>
#include<stdlib.h>

using namespace std;

passwd::passwd()
{
  pwd=(char **)malloc(sizeof(char*));
  pwd[0]=(char *)malloc(9);
  strcpy(pwd[0],"password");
}

passwd::~passwd()
{
   free(pwd[0]);
   free(pwd);
}

passwd& passwd::operator=(const passwd pswd)
{
  pwd[0]=(char *)realloc(pwd[0],strlen(*pswd.pwd)+1);
  strcpy(pwd[0],*pswd.pwd);
  return *this;
}

bool passwd::check_validity()
{
  if(strlen(pwd[0])<=MAX_PASSWD_LEN&&strlen(pwd[0])>=MIN_PASSWD_LEN);
  else
    return false;
    int number_count=0,alphabet_count=0,symbol_count=0;
    for(int i=0;i<strlen(pwd[0]);i++)
     {
        if(pwd[0][i]<='9'&&pwd[0][i]>='0')
         {
            number_count++;
         }
        else if((pwd[0][i]>='a'&&pwd[0][i]<='z')||(pwd[0][i]>='A'&&pwd[0][i]<='Z'))
          {
            alphabet_count++;

          }
        else if(pwd[0][i]>32&&pwd[0][i]!=127)
          {
            symbol_count++;
          }
      }
if(number_count>=NO_OF_NUMBERS&&alphabet_count>=NO_OF_ALPHABETS  && symbol_count>=NO_OF_SYMBOLS)
      return true;
    else
      return false;
}

void passwd::tochar(char **pswd)
{
  *pswd=(char*)malloc(strlen(pwd[0])+1);
  strcpy(*pswd,pwd[0]);
}

void passwd::get()
{
  char *pswd;
  pswd=getpass("");
  pwd[0]=(char *)realloc(pwd[0],strlen(pswd)+1);
  strcpy(pwd[0],pswd);
}

main program: p1.c++

#include"passwd.h"
#include<iostream>
#include<stdlib.h> 
int main()
{
  using namespace std;
  passwd p;
  cout<<"Enter password: ";
  p.get();
  if(p.check_validity())
    cout<<"Valid pwd.\n";
  else
    cout<<"Invalid pwd.\n";
  char *s;
  p.tochar(&s);
  cout<<s<<endl;
  passwd x=p;
  free(s);
  x.tochar(&s);
  cout<<s<<endl;
  free(s);
}

Answer 1

Your passwd class has an implicitly defined copy constructor. If you don't explicitly create a constructor that looks a bit like

passwd(const passwd &other)

then the compiler with automatically generate one which simply copies all the members. So when you do

passwd x=p;

then you effectively do x.pwd = p.pwd;. As a result, when the second passwd gets destructed, you free pwd[0] and pwd a second time.

To solve this, explicitly define a copy constructor that does what you want it to do (probably allocate a new pwd and pwd[0] then copy the password into it). Or, alternately, define a private copy constructor to prevent it from being used. Or if you're using a modern constructor you can explicitly delete the copy constructor with passwd(const passwd &other) = delete; in the class declaration. Note that the latter two options will make passwd x = p; an error.

This breaks even if you do passwd x; x = p; due to another issue: Your operator= does not take a reference to the original, and so a copy is made when invoking it. Usually one would define an operator=(const passwd &pswd) - by leaving out the & a copy of the argument must be made, which is done using the copy constructor, which then causes the double-free after you return from operator= and the copied pswd is destroyed.