Encoding arbitrary data for stream transmission

Question

Basically, what this function should do is encode an arbitrary set of data in memory to a series of bytes that starts with 0x02, ends with 0x03, and encodes each instance of 0x02, 0x03, or 0x25 by preceding it with 0x25. It does this to make it safe for stream transmission, for example, a TCP socket. It returns a non-null-terminated array of char allocated with malloc (and therefore must be freed by the caller) and the length of the new array.

size_t encodedata(char** outdata, const void* data, size_t size)
{
    const char* dchr = data;
    size_t n = 2;
    for (size_t i = 0; i < size; i++)
        n += dchr[i] == 0x02 ? 2 : dchr[i] == 0x03 ? 2 : dchr[i] == 0x25 ? 2 : 1;

    char* encdata = malloc(n);

    encdata[0] = 0x02;
    encdata[n-1] = 0x03;

    for (size_t i = 0, j = 1; i < size && j < (n - 1); i++) {
        if (dchr[i] == 0x02 || dchr[i] == 0x03 || dchr[i] == 0x25)
            encdata[j++] = 0x25;
        encdata[j++] = dchr[i];
    }

    *outdata = encdata;
    return n;
}

The main function, which tests encodedata, runs without problems:

int main(int argc, char* argv[])
{
    char* out;
    char* in = "1" "\x02" "%" "\x03" "3";

    size_t n = encodedata(&out, in, 6);

    printf("size: %d\n", n);

    for (size_t i = 0; i < n; ++i)
        printf("%x ", out[i]);

    printf("\n");

    assert(n == 11);
    assert(out[0] == 0x02);
    assert(out[1] == '1');
    assert(out[2] == 0x25);
    assert(out[3] == 0x02);
    assert(out[4] == 0x25);
    assert(out[5] == 0x25);
    assert(out[6] == 0x25);
    assert(out[7] == 0x03);
    assert(out[8] == '3');
    assert(out[9] == 0x00);
    assert(out[10] == 0x03);

    free(out);
}

Answer 1

Assuming the encoding format itself is not subject to review, it's a pretty good implementation. Things that could be improved (small stuff):

• Chaining ?: is not necessary here, compare:

  n += dchr[i] == 0x02 ? 2 : dchr[i] == 0x03 ? 2 : dchr[i] == 0x25 ? 2 : 1;
  

  to:

  n += (dchr[i] == 0x02 || dchr[i] == 0x03 || dchr[i] == 0x25) ? 2 : 1;
  

• Prefer unsigned char* for work on raw binary sequences, char* means a string of text by convention

• 0x02, 0x03, and 0x25 all have meanings, so better give them meaningful names, e.g.

  const unsigned char ENC_START = 0x02, ENC_END = 0x03, ENC_ESCAPE = 0x25;
  

• In the test:

  char* in = "1" "\x02" "%" "\x03" "3";
  size_t n = encodedata(&out, in, 6);
  

  it's not clear where 6 came from, so I'd opt for:

  char in[] = "1" "\x02" "%" "\x03" "3";
  size_t n = encodedata(&out, in, sizeof(in));
  

Moreover, unless the function signature you proposed is a requirement, I would opt for a buffered design that encodes data in chunks on the fly. It's a little more complicated because you would need to remember the state between calls -- how much input data was processed so far (think of fread). For stream transmission, you probably call encodedata, send the result, and throw it away immediately after. So elliding malloc and relieving the caller from encoding the whole stream at once are both welcome here. One of many possible designs may look like this:

struct encodestate
{
    void* in;
    size_t inlen;
    unsigned char out[256];  // use apprioprate chunk length for your stream
    size_t outlen;
};

/* Writes encoded data into state.out, updates state.outlen
   and for internal use updates state.in and state.inlen.

   Returns whether there's more input data to process (should be called again).
*/
bool encode(struct encodestate* state);

Example of use:

unsigned char in[] = "1" "\x02" "%" "\x03" "3";

for (struct encodestate state = {in, sizeof in}; encode(&state); )
    send(somewhere, state.out, state.outlen);

Answer 2

1. You don't check whether malloc() failed.

2. As you are sure you have enough memory, consider trading some memory for making the algorithm one-pass: The encoded result is at most \$2*size+2\$ long.
   If you aren't quite so sure, consider only pre-calculating the real size if over-allocation fails.

3. Simplify your conditional-operator-chain.

4. Consider changing the calculation of n so you only add the space for the escape-character in the loop. That might be more efficient.

5. Actually, consider not allocating any memory in your function, as it's likely to be called quite often and allocation is both costly and often unneccessary. Instead of passing an out-parameter, pass a buffer and length.

6. char can have a trap representation or a negative zero. Consider working with unsigned char instead, like the standard library does (even if the interface says plain char).

7. Consider using memcmp() for writing your test-case.