Implementation of CString::replace(...) - suggestions sought

Projects Programming
1

For those of you that are particularly good at coding, can I pick your brains?

I have 3 particular versions of CString::replace(...) to cater for all possible combinations of parameters:

void replace(const char* strFind, const char* strReplace, const uint16_t nStartFrom = 0);

#ifndef __SAM3X8E__

  void replace(const char* strFind, const __FlashStringHelper* strReplace, const uint16_t nStartFrom = 0);

  void replace(const __FlashStringHelper* strFind, const char* strReplace, const uint16_t nStartFrom = 0);

  void replace(const __FlashStringHelper* strFind, const __FlashStringHelper* strReplace, const uint16_t nStartFrom = 0);

#endif

Only the first version of this function implements the actual algorithm. The other versions simply copy the strings in flash memory to global memory and then call the call void replace(const char* strFind, const char* strReplace, const uint16_t nStartFrom = 0);

But the problem is the consecutive function calls are loading global memory and it is increasing the risk of memory corruption.

Is there are better way to implement these functions, that has not occurred to me, and that does not load global memory while also not requiring me to have 3 separate chunks of code that I need to change should I need to make changes to the algorithm.

#ifndef __SAM3X8E__
  void CString::replace(const __FlashStringHelper* strFind, const __FlashStringHelper* strReplace, const uint16_t nStartFrom)
  {
    const uint16_t nLength = 256;
    char strTemp1[nLength], strTemp2[nLength];
    
    memset(strTemp1, 0, nLength);
    memset(strTemp2, 0, nLength);
    strcpy_P(strTemp1, (char*)strFind);
    strcpy_P(strTemp2, (char*)strReplace);
    replace(strTemp1, strTemp2, nStartFrom);
  }

  void CString::replace(const char* strFind, const __FlashStringHelper* strReplace, const uint16_t nStartFrom)
  {
    const uint16_t nLength = 256;
    char strTemp[nLength], strTemp2[nLength];
    
    memset(strTemp, 0, nLength);
    strcpy_P(strTemp, (char*)strReplace);
    replace(strFind, strTemp, nStartFrom);
  }

  void CString::replace(const __FlashStringHelper* strFind, const char* strReplace, const uint16_t nStartFrom)
  {
    const uint16_t nLength = 256;
    char strTemp[nLength], strTemp2[nLength];
    
    memset(strTemp, 0, nLength);
    strcpy_P(strTemp, (char*)strFind);
    replace(strTemp, strReplace, nStartFrom);
  }
#endif

void CString::replace(const char* strFind, const char* strReplace, const uint16_t nStartFrom)
{
  uint16_t nLengthMove = 0, nNewLength = 0;
  const uint16_t nLength = 512;
  char strTempBuff[nLength];

  if ((strlen(m_strBuff) - strlen(strFind) + strlen_P((PGM_P)strReplace) + 1) <= m_nBuffSize)
  {
    if (nStartFrom < strlen(m_strBuff))
    {
      m_strStartPtr = m_strBuff + nStartFrom;
      m_strEndPtr = strstr(m_strStartPtr, strFind);
  
      if (m_strEndPtr)
      {
        nNewLength = strlen(m_strBuff) + strlen(strReplace) - strlen(strFind);
        
        if (nNewLength < m_nBuffSize)
        {
          memset(strTempBuff, 0, nLength);
          strncpy(strTempBuff, m_strBuff, m_strEndPtr - m_strStartPtr);
          strcat(strTempBuff, strReplace);
          strcat(strTempBuff, m_strEndPtr + strlen(strFind));
          memset(m_strBuff, 0, m_nBuffSize);
          strcpy(m_strBuff, strTempBuff);
        }
      }
    }
  }
}
2

You allocate your memory on the stack so there is no risk for poking holes in the heap but the challenge with you approach is that you allocate tons of memory even if you require a few bytes

You should just allocated the required amount of memory for your operation, you could also read the data directly from flash without the intermediary buffers

You don't need to 0 the full buffers dithers, just add a trailing 0 if the copy function you used did not do it

3

J-M-L:
You allocate your memory on the stack so there is no risk for poking holes in the heap but the challenge with you approach is that you allocate tons of memory even if you require a few bytes

You should just allocated the required amount of memory for your operation, you could also read the data directly from flash without the intermediary buffers

You don't need to 0 the full buffers dithers, just add a trailing 0 if the copy function you used did not do it

At one stage I was using 'new char[]' and allocating just the amount of memory required. But then using the heap is an issue too.

I am aware of the _P versions of all those functions but then I have to maintain 3 separate sets of code that all do the same job.

I might have to live with the latter unless there are some other Arduino coding tricks I am not ware of.

If there was some way of determining what part of arduino memory a string is residing in then I might be able to use macro to determine which version, _P or non _P, to use.

4

Something like this:

void CString::_replace(const char* strFind, bool findIsFlash, const char* strReplace, bool replaceIsFlash, const uint16_t nStartFrom)

With all your functions simply calling this one with appropriate parameters.