Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 8 of 8

Thread: Issue with returning zero length string from String.substring()

  1. #1
    Junior Member
    Join Date
    May 2021
    Posts
    2

    Issue with returning zero length string from String.substring()

    Executing the following code will result in a hung Teensy on loop 6:

    Code:
    String destination_string;
    String source_string;
    int counter; 
    
    void setup()
    {
      source_string = "ABC";
      destination_string = "123";
      counter = 0;
    }
    
    void loop()
    {
      counter++;
      Serial.print("Beginning loop ");
      Serial.println(counter);
      Serial.send_now();
    
      Serial.print("  source_string is ");
      Serial.println(source_string);
      Serial.send_now();
    
      if(counter > 5)
      {
        destination_string = source_string.substring(2,2);
      }
      Serial.print("  destination_string is ");
      Serial.println(destination_string);
      Serial.send_now();
    
      delay(1000);
    }
    The issue only occurs if the destination string already contains something (i.e. it is greater than zero length), and only occurs if the substring() call returns a string of zero length. We have tested on Teensy 4.0 and Teensy 4.1, both result in a fairly hard hang (requiring power cycle or button push). Interestingly a set watchdog timer will get it restarted on its own (obviously not included in the code above). Code runs fine and forever on regular Arduino/Mega/etc., just need to comment out the Serial.send_now() calls. This was all tested on Arduino 1.8.13, with Teensy 1.5.3 (on Linux).

    We have started to look at the Teensy String code, but it will probably take us a while to muddle through that. Let us know if we can provide any further details on this that might be helpful.

  2. #2
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,111
    I can confirm this - but it doesn't happen on the T3.2 despite the printing and String
    code seeming identical where it should matter.

  3. #3
    Senior Member
    Join Date
    Oct 2019
    Location
    Calgary
    Posts
    121
    At first glance, the culprit seems to be in String::move, which is the function that ends up being called by destination_string = source_string.substring(2,2);. In this case, the lhs has an available buffer, and it's capacity (3) is greater than or equal to the length (0) of the rhs, so the code tries to do a strcpy. Unfortunately, the value of rhs.buffer is NULL.

    Code:
    void String::move(String &rhs)
    {
       if (buffer) {
          if (capacity >= rhs.len) {
             strcpy(buffer, rhs.buffer);
             len = rhs.len;
             rhs.len = 0;
             return;
          } else {
             free(buffer);
          }
       }
       buffer = rhs.buffer;
       capacity = rhs.capacity;
       len = rhs.len;
       rhs.buffer = NULL;
       rhs.capacity = 0;
       rhs.len = 0;
    }
    My initial hunch is that adding a check for a non NULL source buffer might do the trick, but I'd like to hear what others think.

    Code:
    void String::move(String &rhs)
    {
       if (buffer) {
          if (capacity >= rhs.len && rhs.buffer) {
             strcpy(buffer, rhs.buffer);
             len = rhs.len;
             rhs.len = 0;
             return;
          } else {
             free(buffer);
          }
       }
       buffer = rhs.buffer;
       capacity = rhs.capacity;
       len = rhs.len;
       rhs.buffer = NULL;
       rhs.capacity = 0;
       rhs.len = 0;
    }

  4. #4
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,111
    Perhaps:
    Code:
    void String::move(String &rhs)
    {
    	if (buffer) {
    		if (capacity >= rhs.len) {
    			if (rhs.buffer)
    				strcpy(buffer, rhs.buffer);
    			else
    				*buffer = '\0';
    			len = rhs.len;
    			rhs.len = 0;
    			return;
    		} else {
    			free(buffer);
    		}
    	}
    	buffer = rhs.buffer;
    	capacity = rhs.capacity;
    	len = rhs.len;
    	rhs.buffer = NULL;
    	rhs.capacity = 0;
    	rhs.len = 0;
    }
    These String classes really need a high coverage testsuide, there have been several nasty bugs over the years

  5. #5
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,111
    I submitted a bug to the Arduino bug report system about this, WString seems to have been in error like
    this for a while, but no-one noticed as most microcontrollers let you deference NULL without faulting!
    https://github.com/arduino/Arduino/issues/11547

    [ BTW I noticed the WString version of substring sorts the start/end arguments which is pretty unusual (and probably
    a nasty gotcha in fact).

  6. #6
    Junior Member
    Join Date
    May 2021
    Posts
    2
    Quote Originally Posted by MarkT View Post
    [ BTW I noticed the WString version of substring sorts the start/end arguments which is pretty unusual (and probably
    a nasty gotcha in fact).
    We had noticed this argument sorting as well, while testing cases to report the above issue. It does seem odd that substring(4, 1) of "foobar" returns "oob".

  7. #7
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,111
    Rather than '' or 'boo' which could actually be handy in some circumstances. High level languages with exception
    handling normally fix all these awkward corners by throwing an exception.

  8. #8

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •