user77356: I am working on building a serial command receiver. The idea is came from the boot loader. However, I got a very strange problem that I have no idea what could cause it.
As the first screenshot show, the print isn't finished because "===a=" is stuck, like this.

In the code, it should print out and the others.
char* receiveCmd(Stream &_serial, char &cmd_ptr, int &l_ptr) {
  Stream* serial = &_serial;

  if (serial->available() && (uint8_t)serialRead(_serial) == CMD_START) {
    int buf_count = 0; //start at data
    cmd_ptr = serialRead(_serial); //cmd_byte

    char low_byte = serialRead(_serial);
    char high_byte = serialRead(_serial);

    uint16_t length = low_byte; //data_length
    length = length | (high_byte << 8);
    

    Serial.println("\n\n===a===");
    Serial.println(length);
    Serial.println("===b===");

    l_ptr = (int)length;

    char* _buf = (char*)malloc(length * sizeof(char));

    while (buf_count != length) {
      if (serial->available()) {
        _buf[buf_count] = serial->read();
        buf_count++;
      }
    }

    char checksum = serialRead(_serial); //checksum, don't bother it yett
    char stop_byte = serialRead(_serial);

    if (stop_byte != CMD_EOF) {
      free(_buf);
      return NULL; //error
    }

    return _buf;
  }

  return NULL;
}

Anyway, the problem is fixed when I print out other variables first, then the length var..
char* receiveCmd(Stream &_serial, char &cmd_ptr, int &l_ptr) {
  Stream* serial = &_serial;

  if (serial->available() && (uint8_t)serialRead(_serial) == CMD_START) {
    int buf_count = 0; //start at data
    cmd_ptr = serialRead(_serial); //cmd_byte

    char low_byte = serialRead(_serial);
    char high_byte = serialRead(_serial);

    uint16_t length = low_byte; //data_length
    length = length | (high_byte << 8);
    

    Serial.println("\n\n===a===");
    Serial.println(low_byte, HEX);
    Serial.println(high_byte, HEX);
    Serial.println(length);
    Serial.println("===b===");

    l_ptr = (int)length;

    char* _buf = (char*)malloc(length * sizeof(char));

    while (buf_count != length) {
      if (serial->available()) {
        _buf[buf_count] = serial->read();
        buf_count++;
      }
    }

    char checksum = serialRead(_serial); //checksum, don't bother it yett
    char stop_byte = serialRead(_serial);

    if (stop_byte != CMD_EOF) {
      free(_buf);
      return NULL; //error
    }

    return _buf;
  }

  return NULL;
}

Other code is exactly the same. Does anyone know what may cause it? I can't print out those thing during production because it is only for debugging. If I remove those print, the code is also stuck(because of the length variable again).
For additional information, the serial signal is generated from other Arduino and AltSerial Library. The structure is like this.
void sendCmd(Stream &_serial, char cmd, char* payload, int length) {
  Stream* serial = &_serial;
  char buf[6 + length]; //start, data_length, data, checksum, stop

  buf[0] = CMD_START; //start_byte
  buf[1] = cmd; //cmd_byte
  buf[2] = length & 0xff; //data_length - low byte
  buf[3] = (length >> 8) & 0xff; //data_length - high byte
  buf[4 + length] = (char)calcCRC(payload); //checksum
  buf[5 + length] = CMD_EOF; //stop_byte

  for (int i = 0; i < length; i++) //load buf
    buf[4 + i] = payload[i];

  for (int i = 0; i < sizeof(buf); i++)
    serial->print(buf[i]);
}

I have used Serial.print(altSerial.read(), HEX) to check whether the byte is received correctly. They are all correct. That's why it is confusing me.QAQ
===== UPDATED INFORMATION =====
I have narrowed the problem. It seems like the below code is the main cause regard the issue. But no solution for this still.
uint16_t length = low_byte; //data_length
length = length | (high_byte << 8);

Thank you.

user7508: Why it does not work? You already got the answer in comments:

You are reading more bytes from the serial stream than what you know
are available. Then, your length variable can contain garbage, which
makes malloc() fail even if you have plenty of available memory.

You write into the memory buffer even if the allocation failed, which
makes your program corrupt its memory. From this point, anything can
happen.


You may notice that the behavior of your program depends on the timing.
Adding more instructions, or delays, may make it work. This is simply
because you give your peer more time to send its data packet. Of course,
relying on the timings is not a good approach: you instead have to make
sure you never read() more than what is actually available().
Here is an (untested) attempt to do this correctly and in a non-blocking
fashion. It is based on a finite state machine. What is original
about this state machine is that it is implemented as a switch/case
where every single case falls through:
/*
 * Packet format:
 *   CMD_START cmd_byte length_L length_H [payload] checksum CMD_EOF
 */
char *receiveCmd(Stream &serial, char &cmd_ref, int &l_ref) {

    // The current state is defined by what parts of the packet
    // we have already received.
    static enum {
        GOT_NONE, GOT_START, GOT_HEADER, GOT_PAYLOAD
    } state = GOT_NONE;

    // Variables only valid in the states GOT_HEADER and GOT_PAYLOAD.
    static char cmd_byte;
    static uint16_t length;
    static char *buffer;
    static size_t buffer_pos;

    switch (state) {
        case GOT_NONE:
            if (serial.available() < 1 || serial.read() != CMD_START)
                break;
            state = GOT_START;
            // fallthrough
        case GOT_START:
            if (serial.available() < 3)
                break;
            cmd_byte = serial.read();
            length = serial.read();
            length |= (uint16_t) serial.read() << 8;
            buffer = (char *) malloc(length);
            if (!buffer) {
                Serial.print(F("ERROR: Failed to allocate "));
                Serial.print(length);
                Serial.println(F(" bytes."));
                state = GOT_NONE;
                break;
            }
            buffer_pos = 0;
            state = GOT_HEADER;
            // fallthrough
        case GOT_HEADER:
            while (buffer_pos < length && serial.available())
                buffer[buffer_pos++] = serial.read();
            if (buffer_pos < length)
                break;
            state = GOT_PAYLOAD;
            // fallthrough
        case GOT_PAYLOAD:
            if (serial.available() < 2)
                break;
            serial.read();  // consume checksum
            if (serial.read() != CMD_EOF) {
                Serial.println(F("ERROR: Expected CMD_EOF."));
                free(buffer);
                state = GOT_NONE;
                break;
            }
            cmd_ref = cmd_byte;
            l_ref = length;
            state = GOT_NONE;
            return buffer;
    }
    return NULL;
}

You can think of this switch/case as a set of actions to perform
sequentially in order to process the whole packet. However, if the
function gets stuck for lack of incoming data, it just bails out. On the
next call, the switch puts it back where it left.

77356: <p>I am working on building a serial command receiver. The idea is came from the boot loader. However, I got a very strange problem that I have no idea what could cause it.</p>
<p>As the first screenshot show, the print isn't finished because &quot;===a=&quot; is stuck, like this.
<a href="https://i.stack.imgur.com/SSrLJ.png" rel="nofollow noreferrer"><img src="https://i.stack.imgur.com/SSrLJ.png" alt="enter image description here" /></a></p>
<p>In the code, it should print out and the others.</p>
<pre><code>char* receiveCmd(Stream &amp;_serial, char &amp;cmd_ptr, int &amp;l_ptr) {
  Stream* serial = &amp;_serial;

  if (serial-&gt;available() &amp;&amp; (uint8_t)serialRead(_serial) == CMD_START) {
    int buf_count = 0; //start at data
    cmd_ptr = serialRead(_serial); //cmd_byte

    char low_byte = serialRead(_serial);
    char high_byte = serialRead(_serial);

    uint16_t length = low_byte; //data_length
    length = length | (high_byte &lt;&lt; 8);
    

    Serial.println(&quot;\n\n===a===&quot;);
    Serial.println(length);
    Serial.println(&quot;===b===&quot;);

    l_ptr = (int)length;

    char* _buf = (char*)malloc(length * sizeof(char));

    while (buf_count != length) {
      if (serial-&gt;available()) {
        _buf[buf_count] = serial-&gt;read();
        buf_count++;
      }
    }

    char checksum = serialRead(_serial); //checksum, don't bother it yett
    char stop_byte = serialRead(_serial);

    if (stop_byte != CMD_EOF) {
      free(_buf);
      return NULL; //error
    }

    return _buf;
  }

  return NULL;
}
</code></pre>
<p>Anyway, the problem is fixed when I print out other variables first, then the length var..</p>
<pre><code>char* receiveCmd(Stream &amp;_serial, char &amp;cmd_ptr, int &amp;l_ptr) {
  Stream* serial = &amp;_serial;

  if (serial-&gt;available() &amp;&amp; (uint8_t)serialRead(_serial) == CMD_START) {
    int buf_count = 0; //start at data
    cmd_ptr = serialRead(_serial); //cmd_byte

    char low_byte = serialRead(_serial);
    char high_byte = serialRead(_serial);

    uint16_t length = low_byte; //data_length
    length = length | (high_byte &lt;&lt; 8);
    

    Serial.println(&quot;\n\n===a===&quot;);
    Serial.println(low_byte, HEX);
    Serial.println(high_byte, HEX);
    Serial.println(length);
    Serial.println(&quot;===b===&quot;);

    l_ptr = (int)length;

    char* _buf = (char*)malloc(length * sizeof(char));

    while (buf_count != length) {
      if (serial-&gt;available()) {
        _buf[buf_count] = serial-&gt;read();
        buf_count++;
      }
    }

    char checksum = serialRead(_serial); //checksum, don't bother it yett
    char stop_byte = serialRead(_serial);

    if (stop_byte != CMD_EOF) {
      free(_buf);
      return NULL; //error
    }

    return _buf;
  }

  return NULL;
}
</code></pre>
<p>Other code is exactly the same. Does anyone know what may cause it? I can't print out those thing during production because it is only for debugging. If I remove those print, the code is also stuck(because of the length variable again).</p>
<p>For additional information, the serial signal is generated from other Arduino and AltSerial Library. The structure is like this.</p>
<pre><code>void sendCmd(Stream &amp;_serial, char cmd, char* payload, int length) {
  Stream* serial = &amp;_serial;
  char buf[6 + length]; //start, data_length, data, checksum, stop

  buf[0] = CMD_START; //start_byte
  buf[1] = cmd; //cmd_byte
  buf[2] = length &amp; 0xff; //data_length - low byte
  buf[3] = (length &gt;&gt; 8) &amp; 0xff; //data_length - high byte
  buf[4 + length] = (char)calcCRC(payload); //checksum
  buf[5 + length] = CMD_EOF; //stop_byte

  for (int i = 0; i &lt; length; i++) //load buf
    buf[4 + i] = payload[i];

  for (int i = 0; i &lt; sizeof(buf); i++)
    serial-&gt;print(buf[i]);
}
</code></pre>
<p>I have used <code>Serial.print(altSerial.read(), HEX)</code> to check whether the byte is received correctly. They are all correct. That's why it is confusing me.QAQ</p>
<p>===== UPDATED INFORMATION =====</p>
<p>I have narrowed the problem. It seems like the below code is the main cause regard the issue. But no solution for this still.</p>
<pre><code>uint16_t length = low_byte; //data_length
length = length | (high_byte &lt;&lt; 8);
</code></pre>
<p>Thank you.</p>
 7508: <p>Why it does not work? You already got the answer in comments:</p>
<ul>
<li><p>You are reading more bytes from the serial stream than what you know
are available. Then, your <code>length</code> variable can contain garbage, which
makes <code>malloc()</code> fail even if you have plenty of available memory.</p>
</li>
<li><p>You write into the memory buffer even if the allocation failed, which
makes your program corrupt its memory. From this point, anything can
happen.</p>
</li>
</ul>
<p>You may notice that the behavior of your program depends on the timing.
Adding more instructions, or delays, may make it work. This is simply
because you give your peer more time to send its data packet. Of course,
relying on the timings is not a good approach: you instead have to make
sure you never <code>read()</code> more than what is actually <code>available()</code>.</p>
<p>Here is an (untested) attempt to do this correctly and in a non-blocking
fashion. It is based on a <a href="https://majenko.co.uk/blog/finite-state-machine" rel="nofollow noreferrer">finite state machine</a>. What is original
about this state machine is that it is implemented as a <code>switch</code>/<code>case</code>
where <em>every single case</em> falls through:</p>
<pre class="lang-cpp prettyprint-override"><code>/*
 * Packet format:
 *   CMD_START cmd_byte length_L length_H [payload] checksum CMD_EOF
 */
char *receiveCmd(Stream &amp;serial, char &amp;cmd_ref, int &amp;l_ref) {

    // The current state is defined by what parts of the packet
    // we have already received.
    static enum {
        GOT_NONE, GOT_START, GOT_HEADER, GOT_PAYLOAD
    } state = GOT_NONE;

    // Variables only valid in the states GOT_HEADER and GOT_PAYLOAD.
    static char cmd_byte;
    static uint16_t length;
    static char *buffer;
    static size_t buffer_pos;

    switch (state) {
        case GOT_NONE:
            if (serial.available() &lt; 1 || serial.read() != CMD_START)
                break;
            state = GOT_START;
            // fallthrough
        case GOT_START:
            if (serial.available() &lt; 3)
                break;
            cmd_byte = serial.read();
            length = serial.read();
            length |= (uint16_t) serial.read() &lt;&lt; 8;
            buffer = (char *) malloc(length);
            if (!buffer) {
                Serial.print(F(&quot;ERROR: Failed to allocate &quot;));
                Serial.print(length);
                Serial.println(F(&quot; bytes.&quot;));
                state = GOT_NONE;
                break;
            }
            buffer_pos = 0;
            state = GOT_HEADER;
            // fallthrough
        case GOT_HEADER:
            while (buffer_pos &lt; length &amp;&amp; serial.available())
                buffer[buffer_pos++] = serial.read();
            if (buffer_pos &lt; length)
                break;
            state = GOT_PAYLOAD;
            // fallthrough
        case GOT_PAYLOAD:
            if (serial.available() &lt; 2)
                break;
            serial.read();  // consume checksum
            if (serial.read() != CMD_EOF) {
                Serial.println(F(&quot;ERROR: Expected CMD_EOF.&quot;));
                free(buffer);
                state = GOT_NONE;
                break;
            }
            cmd_ref = cmd_byte;
            l_ref = length;
            state = GOT_NONE;
            return buffer;
    }
    return NULL;
}
</code></pre>
<p>You can think of this <code>switch</code>/<code>case</code> as a set of actions to perform
sequentially in order to process the whole packet. However, if the
function gets stuck for lack of incoming data, it just bails out. On the
next call, the <code>switch</code> puts it back where it left.</p>
