0

Function called by object output of same class as object input seems to cause unexpected behaviour and overwrite object input's mallocated private member's data (pointer adresses stays same)

For object output both *fileStr and *p_file are NULL and for input both points at data
Both CASE1/CASE2 or combination of each #ifdef will cause input.fileStr data to be changed
input.fileStr data itself is malloc'ated by class1::open if called (called only by input) - else it's a NULL pointer by default

Header

class class1
{
private:
    FILE *p_file = NULL;
    char *fileStr = NULL;
    bool encrypt_step1();
public:
    bool open(char *pathto);
    bool create(char *pathto);
    bool encrypt();
    ~class1();
};
bool sub_function(char *pathIN);

Source Code

bool class1::open(char *pathto)
{
    if (PathFileExistsA(pathto))
        this->p_file = fopen(pathto, "rb+");
    else
        return 0;
    if (!(this->p_file))
    {
        printf("Can't open\n");
        return 0;
    }
    fseek(p_file, 0, SEEK_END);
    long filesize = ftell(p_file);
    fseek(p_file, 0, SEEK_SET);
    this->fileStr = (char*)malloc(filesize+1);
    this->fileStr[(fread(this->fileStr, 1, filesize, this->p_file))] = '\0';
    return 1;
}
bool class1::create(char *pathto)
{
#ifdef CASE1
    if (PathFileExistsA(pathto))
    {
        char pathtobak[MAX_PATH];
        strcpy(pathtobak, pathto);
        strcat(pathtobak, ".bak");
        int i = 0;
        char a[11];
        if (PathFileExistsA(pathtobak))
        {
            while (1)
            {
                i++;
                *a = '\0';
                itoa(i, a, 10);
                char *reset = pathtobak + strlen(pathtobak);
                strcat(pathtobak, a);
                if (!PathFileExistsA(pathtobak))
                    break;
                *reset = '\0';
            }
        }
        std::experimental::filesystem::copy_file(pathto, pathtobak);
    }
#endif
#ifdef CASE2
    this->p_file = fopen(pathto, "wb");
#endif
#ifndef NOERRORS
    if (this->p_file == NULL)
        return 0;
    else
        return 1;
#endif
}
class1::~class1()
{
    if (this->fileStr)
    {
        free(this->fileStr);
        this->fileStr = NULL;
    }
    if (this->p_file)
    {
        fclose(this->p_file);
        this->p_file = NULL;
    }
}
bool sub_function(char *pathIN)
{
    class1 input;
    input.open(pathIN);
    input.encrypt();//omitted since this should be irrelevant
    char pathOUT[MAX_PATH];
    strcpy(pathOUT, pathIN);
    char *OUText = pathOUT;
    OUText += (strlen(pathOUT)-3);
    *OUText = '\0';
    strcat(pathOUT, "ext");
    class1 output;
    output.create(pathOUT);//bug here
    char *next = input.get_fileStr();
    ...
}

It seems like memory access violation, but even CASE1 that's simple look up for duplicate files with only use of local variable still causes unexpected behaviour, so I have issues pinpointing the cause

Memory seems to be already released

Most plausible cause would be memory being marked as free, but I don't deallocate it outside of destructor, but when running program further once destructor of input has been called free(input.fileStr) will crash with is_block_type_valid(header->_block_use) exception

Actual issue with working example

Source Code starting from sub_function::input.encrypt()

bool class1::encrypt_step1()
{
    bool *strBOOL_11_00 = (bool*)malloc(((strlen(this->fileStr)) + 1) * ((sizeof(bool)) * 8));
    bool *strBOOL_10_01 = (bool*)malloc(((strlen(this->fileStr)) + 1) * ((sizeof(bool)) * 8));
    bool *strBOOL_10_11 = (bool*)malloc(((strlen(this->fileStr)) + 1) * ((sizeof(bool)) * 8));
    char *fileStrIt = this->fileStr;
    char *fileStrIt2 = ((this->fileStr) + 1);
    bool *next1100 = strBOOL_11_00;
    bool *next1001 = strBOOL_10_01;
    bool *next1011 = strBOOL_10_11;

    //translating to binary array iterating through pointers above one by one happens here
    //char->bin/encrypt/bin->char
    //ommited

    //reallocation to fit new encrypted and translated back to char that caused issues
#ifdef ISSUE
    char *fileStr_temp = (char *)realloc(this->fileStr, ((next1011 - strBOOL_10_11) + (next1001 - strBOOL_10_01) + (next1100 - strBOOL_11_00) + 1));
    if (!fileStr_temp)
        return 0;
    //original fileStr points at freed memory
#endif

#ifdef CORRECT
    char *fileStr_temp = (char *)realloc(this->fileStr, ((next1011 - strBOOL_10_11) + (next1001 - strBOOL_10_01) + (next1100 - strBOOL_11_00) + 1));
    if (!fileStr_temp)
        return 0;
    else
        this->fileStr = fileStr_temp;//original fileStr points at new adress with reallocated data
#endif
    free(strBOOL_11_00);
    strBOOL_11_00 = NULL;
    free(strBOOL_10_01);
    strBOOL_10_01 = NULL;
    free(strBOOL_10_11);
    strBOOL_10_11 = NULL;
    return 1;
}
bool class1::encrypt()
{
    encrypt_step1();
    ...//other steps (irrelevant)
    return 1;
}
namonaki
  • 53
  • 5
  • Without a [mcve], it is not possible to answer your question. There is nothing obviously wrong with the code that's shown, therefore the problem must happen in the code that's not shown, and it's unlikely that anyone can tell you what's wrong in the code that's not shown. This doesn't mean that you need to post all of your code, but only what's needed to provide a [mcve], as explained in stackoverflow.com's [help]. Having said that, the most likely answer is that the shown class does not comply with the [Rule Of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). – Sam Varshavchik Jun 16 '19 at 23:05
  • Thing is only input.encrypt() has been ommited - all other ommited functions are being called after bug occurs and has been disabled for sake of debugging anyway - input.encrypt() doesn't modify ```class1.fileStr``` at current stage anyway.
    But if this is something that can be fixed with full code I could edit it a little and make a full example out of it - thought that would be an overkill. Will post full code later if nothing obvious suffices.
    – namonaki Jun 16 '19 at 23:18
  • No, I'm confident it's more than "only input.encrypt()" that has been ommitted. And, because I was pretty clear that "this doesn't mean that you need to post all of your code", and then you proceeded to promise "post full code": that shows that you are ignoring the advice and help that others are willing to provide you, so I don't see why anyone would want to help you. What part of a "[mcve]" are you unclear about? – Sam Varshavchik Jun 16 '19 at 23:27
  • In `sub_function`, names `path` and `inputTXT` are used that were never declared. This suggests that the code you show is not the code you are actually running. – Igor Tandetnik Jun 17 '19 at 00:03
  • Sorry - I misread your comment and actually meant "I'll add encrypt part(after clean up) to code example", not just whole thing. But thanks to your first answer I reviewed ```class1.encrypt()``` instead (since it has access to ```class1.fileStr```) and Rule of Three was an issue - I used ```realloc()``` to resize fileStr memory block, but since it may return NULL on failure I used temporary pointer ```fileStr_temp``` that than I forgot to copy adress value of on success, to original ```fileStr``` Going to add actual core of issue to main post and working implemantation - thank you both! – namonaki Jun 17 '19 at 09:10
  • Igor Tandetnik These were typos I forgot to rename when making example - fixed. Also code with actual place that was core of an issue posted - not sure if it's clear enough, what was causing this issue for me (for people having same issues in future). Code is fully working again with ```#define CORRECT``` since issue layed at not passing a new pointer. Thank you again – namonaki Jun 17 '19 at 09:36

0 Answers0