dstroy0 / InputHandler

Arduino input handler
https://dstroy0.github.io/InputHandler/
GNU General Public License v3.0
1 stars 0 forks source link

dynamic allocation portability #50

Closed dstroy0 closed 2 years ago

dstroy0 commented 2 years ago

replace new with malloc or calloc, and delete or delete[] with free

I've already pushed some of this, the rest of the operators need to be replaced to ensure portability.

dstroy0 commented 2 years ago

Something undefined is happening that I need to chase down with freeing split_input.

dstroy0 commented 2 years ago

The problem is in UserInput::_splitZDC()

dstroy0 commented 2 years ago

this is how I set the function up for debug:

inline bool UserInput::_splitZDC(_rcfbprm& rprm, const size_t num_zdc, const CommandParameters** zdc)
{
    InputProcessDelimiterSequences pdelimseq;
    memcpy_P(&pdelimseq, _input_prm_.pdelimseq, sizeof(pdelimseq));
    for (size_t i = 0; i < num_zdc; ++i) // look for zero delim commands and put a delimiter between the command and data
    {
        size_t cmd_len_pgm = pgm_read_dword(&(zdc[i]->command_length));       // read command len from CommandParameters object
        if (memcmp_P(rprm.input_data, zdc[i]->command, cmd_len_pgm) == false) // match zdc
        {
            char* ptr = (char*)rprm.split_input;

            memcpy(rprm.split_input, rprm.input_data, cmd_len_pgm); // copy the command into token buffer
            Serial.println(strlen((char*)rprm.split_input));
            Serial.println((char*)rprm.split_input);
            if (rprm.split_input != NULL)
            {
                Serial.println(F("free split_input"));
                ptr = NULL;
                free(rprm.split_input);
                rprm.split_input = NULL;
            }
            ptr = (char*)rprm.split_input + cmd_len_pgm;
            memcpy(ptr, pdelimseq.delimiter_sequences[0], pdelimseq.delimiter_lens[0]); // copy the delimiter into token buffer after the command
            Serial.println(strlen((char*)rprm.split_input));
            Serial.println((char*)rprm.split_input);
            ptr = (char*)rprm.split_input + cmd_len_pgm + pdelimseq.delimiter_lens[0];
            memcpy(ptr, rprm.input_data + cmd_len_pgm, (rprm.input_len - cmd_len_pgm)); // copy the data after the command and delimiter into token buffer
            Serial.println(strlen((char*)rprm.split_input));
            Serial.println((char*)rprm.split_input);
            return true;
        }
    }
    return false;
}

I move the free() if statement down and see where I am introducing undefined behavior. Where it is now I get a "StoreProhibited" kernel panic on esp, which is good, because it's not corrupting the heap. If I move it to the end I write over something with one of the memcpy() (introduce undefined behavior) I suspect and I get a corrupt heap.

2bndy5 commented 2 years ago

I don't see any obvious misuse. Does it work without the Serial.println((char*)rprm.split_input); statements? This question is a complete guess, but I have seen silent fatal errors when printing out c-strings with no null terminator.

dstroy0 commented 2 years ago

rprm.input_len - cmd_len_pgm is not right

dstroy0 commented 2 years ago

It's another sizing issue.

dstroy0 commented 2 years ago

It's copying 81 bytes into a 72 byte buffer, which breaks free()

dstroy0 commented 2 years ago

I'm trying to figure out what is happening with rprm.input_len

2bndy5 commented 2 years ago

That var seems to be set outside the scope of the given snippet.

dstroy0 commented 2 years ago

Ok, I think I understand what is happening and am working on a fix.

dstroy0 commented 2 years ago

This works:

inline bool UserInput::_splitZDC(_rcfbprm& rprm, const size_t num_zdc, const CommandParameters** zdc)
{
    if (num_zdc != 0) // if there are zero delim commands
    {
        InputProcessDelimiterSequences pdelimseq;
        uint8_t delim_len = pgm_read_byte(&_input_prm_.pdelimseq->delimiter_lens[0]);
        size_t split_input_len = rprm.input_len + delim_len + 1U;            

        Serial.print(F("input buffer size in bytes: "));
        Serial.println(rprm.input_len);
        Serial.print(F("split_input buffer size in bytes: "));
        Serial.println(split_input_len);
        rprm.split_input = (uint8_t*)calloc(split_input_len, sizeof(uint8_t));
        if (rprm.split_input == NULL) // if there was an error allocating the memory
        {
            #if defined(__DEBUG_READCOMMANDFROMBUFFER__)
            UserInput::_ui_out(PSTR(">%s$ERROR: cannot allocate ram to split input for zero delim command.\n"), (char*)pgm_read_dword(_input_prm_.pname));
            #endif
            return false;
        }
        memcpy_P(&pdelimseq, _input_prm_.pdelimseq, sizeof(pdelimseq));
        for (size_t i = 0; i < num_zdc; ++i) // look for zero delim commands and put a delimiter between the command and data
        {
            size_t cmd_len_pgm = pgm_read_dword(&(zdc[i]->command_length));       // read command len from CommandParameters object
            if (memcmp_P(rprm.input_data, zdc[i]->command, cmd_len_pgm) == false) // match zdc
            {
                char* ptr = (char*)rprm.split_input;

                memcpy(rprm.split_input, rprm.input_data, cmd_len_pgm); // copy the command into token buffer
                Serial.println(strlen((char*)rprm.split_input));
                Serial.println((char*)rprm.split_input);

                ptr = (char*)rprm.split_input + cmd_len_pgm;
                memcpy(ptr, pdelimseq.delimiter_sequences[0], pdelimseq.delimiter_lens[0]); // copy the delimiter into token buffer after the command
                Serial.println(strlen((char*)rprm.split_input));
                Serial.println((char*)rprm.split_input);

                ptr = (char*)rprm.split_input + cmd_len_pgm + pdelimseq.delimiter_lens[0];
                char* src = (char*)rprm.input_data + cmd_len_pgm;
                Serial.print(F("src: "));
                Serial.println(src);
                Serial.print(F("memcpy size: "));
                Serial.println(rprm.input_len - delim_len);
                memcpy(ptr, src, (rprm.input_len - delim_len)); // copy the data after the command and delimiter into token buffer
                Serial.println(strlen((char*)rprm.split_input));
                Serial.println((char*)rprm.split_input);                
                rprm.token_buffer_len++;
                rprm.input_len = split_input_len;
                return true;
            }
        }
    }
    return false;
}

I don't think I need the src ptr, the problem was that I was writing outside of allocated memory with that last memcpy() and then trying to free() which corrupted the heap.