Parse a contrived message protocol with first byte data length followed by data

I think this code to parse a contrived message protocol with first byte data length followed by data is a little ugly. Has anyone got any suggestions on how to make it more elegant and more robust?

```
#include
#include
#include

#define LENGTHBYTES 1

void splitmessages(unsigned char* bstream, int length);
void printmsg(unsigned char* msg);

int main() {

unsigned char stream[] = {2, 'A', 'A', 1, 'B', 3, 'C', 'C', 'C', 1, 'D' };
int len = sizeof(stream) / sizeof(stream[0]);

printf("\nTest 1 - four messages together\n");
splitmessages(stream, len); //works

printf("\nTest 2 - two complete msgs, then 2 complete msgs\n");
splitmessages(stream, len-2); //works
splitmessages(stream+9, 2);

//split across a message - in data
printf("\nTest 3 - messages split across data part\n");
splitmessages(stream, len-4); //works
splitmessages(stream+7, 4);

//split across a message - just after size
printf("\nTest 4 - messages split just after size part\n");
splitmessages(stream, len-5);
splitmessages(stream+6, 5);

//split across a message - just before size
printf("\nTest 5 - messages split just before size part\n");
splitmessages(stream, len-6);
splitmessages(stream+5, 6);

return 0;
}

void splitmessages(unsigned char* bstream, int length) {
//Create msg as soon as you have size and save as static
static unsigned char* msg = 0;
static int tempbufposition = 0;
static int tocompletemsg = 0;

enum STAGE { SIZE, DATA };
STAGE stage = SIZE;
int size = 0;

//adjust stage to DATA if already have start of msg
if (tocompletemsg != 0)
stage = DATA;

//get cached size
if(tempbufposition != 0 && msg)
size = msg[0];

for(int i=0; i<length; ++i){
switch(stage) {
case SIZE:
size = bstream[i];
if(msg) {
free(msg);
msg = 0;
tempbufposition = 0;
tocompletemsg = 0;
}
//re-create complete ms

Below is an alternative example of how messages could be extracted. Instead of a state machine with saved-state inside your splitmessages function, I have moved the saved-state out of the function into a structure that is passed-in. I have also checked for unexpected data at the start of the function. The return value is 0 on failure or the number of bytes consumed from the input stream on success. I didn't check for size bytes occurring in the wrong place - replace the memcpy with a function that checks the bytes copied for that.

Note that your protocol has to distinguish between data bytes and size bytes. I have arbitrarily made the boundary 32, the first printable char.

#include 
#include 

#define DATA_MIN_VALUE 32
#define DATA_MAX_SIZE  (DATA_MIN_VALUE)

struct message {
    size_t size;
    size_t remaining;
    char data[DATA_MAX_SIZE];
};  
typedef struct message Message;

static inline size_t min(size_t a, size_t b)
{
    return a = DATA_MIN_VALUE;
}

static size_t
get_message(Message *msg, const unsigned char *bstream, size_t length)
{
    size_t len;
    assert(length);

    if ((msg->remaining == 0) && is_data(bstream[0])) {
        /* Data instead of message; caller must handle - eg. discard data up
         * to next message */
        return 0;
    }

    if (is_data(bstream[0])) {
        msg->size = 0;
        msg->remaining = bstream[0];
    }

    len = min(length-1, msg->remaining);

    memcpy(&msg->data[msg->size], &bstream[1], len);
    msg->size += len;
    msg->remaining -= len;

    return len + 1;
}

The code should compile but is untested. Hope it helps to have another perspective.

StackExchange Code Review Q#16269, answer score: 2