ec_encode_data_update gives wrong result when output buffers are not zero-initialized

Hi all,

I was writing an example program to show how to use `ec_encode_data_update` when I noticed that `ec_encode_data` seems to not care whether the output buffers passed into it are zero-initialized, while on the other hand `ec_encode_data_update` does seem to care and will give an incorrect result if the output buffers passed in to it are not zero-initialized.

Here is a program that demonstrates the problem (you can copy-paste it into `ec_simple_example.c` and compile it with `make ex`):

```c
#include
#include
#include
#include
#include "erasure_code.h" // use instead when linking against installed

#define MMAX 255
#define KMAX 255

typedef unsigned char u8;

static int gf_gen_decode_matrix_simple(u8 * encode_matrix,
u8 * decode_matrix,
u8 * invert_matrix,
u8 * temp_matrix,
u8 * decode_index,
u8 * frag_err_list, int nerrs, int k, int m);

int main(int argc, char *argv[])
{
int i, j, m, ret;
int k = 200, p = 50, len = 8 * 1024; // Default params
int nerrs = 0;

// Fragment buffer pointers
u8 *frag_ptrs[MMAX];
u8 *recover_srcs[KMAX];
u8 *recover_outp[KMAX];
u8 *recover_outp_encode_update[KMAX];
u8 frag_err_list[MMAX];

// Coefficient matrices
u8 *encode_matrix, *decode_matrix;
u8 *invert_matrix, *temp_matrix;
u8 *g_tbls;
u8 decode_index[MMAX];

if (argc == 1)
for (i = 0; i < p; i++)
frag_err_list[nerrs++] = rand() % (k + p);

m = k + p;

srand(4); // picked 4 as the seed
k = (rand() % (MMAX - 1)) + 1; // Pick k {1 to MMAX - 1}
p = (rand() % (MMAX - k)) + 1; // Pick p {1 to MMAX - k}

for (i = 0; i < k + p && nerrs < p; i++)
if (rand() & 1)
frag_err_list[nerrs++] = i;

// Check for valid parameters
if (m > MMAX || k > KMAX || m < 0 || p < 1 || k < 1) {
printf(" Input test parameter error m=%d, k=%d, p=%d, erasures=%d\n",
m, k, p, nerrs);
}
if (nerrs > p) {
printf(" Number of erasures chosen exceeds power of code erasures=%d p=%d\n",
nerrs, p);
}
for (i = 0; i < nerrs; i++) {
if (frag_err_list[i] >= m) {
printf(" fragment %d not in range\n", frag_err_list[i]);
}
}

printf("ec_simple_example:\n");

// Allocate coding matrices
encode_matrix = malloc(m * k);
decode_matrix = malloc(m * k);
invert_matrix = malloc(m * k);
temp_matrix = malloc(m * k);
g_tbls = malloc(k * p * 32);

if (encode_matrix == NULL || decode_matrix == NULL
|| invert_matrix == NULL || temp_matrix == NULL || g_tbls == NULL) {
printf("Test failure! Error with malloc\n");
return -1;
}
// Allocate the src & parity buffers
for (i = 0; i < m; i++) {
if (NULL == (frag_ptrs[i] = malloc(len))) {
printf("alloc error: Fail\n");
return -1;
}
}

// Allocate buffers for recovered data
for (i = 0; i < p; i++) {
if (NULL == (recover_outp[i] = malloc(len))) {
printf("alloc error: Fail\n");
return -1;
}
if (NULL == (recover_outp_encode_update[i] = malloc(len))) {
printf("alloc error: Fail\n");
return -1;
}
}
// Fill recover_outp with random data
for (i = 0; i < p; i++)
for (j = 0; j < len; j++){
recover_outp[i][j] = rand();
recover_outp_encode_update[i][j] = rand();
}

// Fill sources with random data
for (i = 0; i < k; i++)
for (j = 0; j < len; j++)
frag_ptrs[i][j] = rand();

printf(" encode (m,k,p)=(%d,%d,%d) len=%d\n", m, k, p, len);

// Pick an encode matrix. A Cauchy matrix is a good choice as even
// large k are always invertable keeping the recovery rule simple.
gf_gen_cauchy1_matrix(encode_matrix, m, k);

// Initialize g_tbls from encode matrix
ec_init_tables(k, p, &encode_matrix[k * k], g_tbls);

// Generate EC parity blocks from sources
ec_encode_data(len, k, p, g_tbls, frag_ptrs, &frag_ptrs[k]);

if (nerrs <= 0)
return 0;

printf(" recover %d fragments\n", nerrs);

// Find a decode matrix to regenerate all erasures from remaining frags
ret = gf_gen_decode_matrix_simple(encode_matrix, decode_matrix,
invert_matrix, temp_matrix, decode_index,
frag_err_list, nerrs, k, m);
if (ret != 0) {
printf("Fail on generate decode matrix\n");
return -1;
}
// Pack recovery array pointers as list of valid fragments
for (i = 0; i < k; i++)
recover_srcs[i] = frag_ptrs[decode_index[i]];

// Recover data
ec_init_tables(k, nerrs, decode_matrix, g_tbls);
ec_encode_data(len, k, nerrs, g_tbls, recover_srcs, recover_outp);

for (i = 0; i < k; i++){
ec_encode_data_update(len, k, nerrs, i, (const u8*)g_tbls, (const u8*)recover_srcs[i], recover_outp_encode_update);
}

// Check that recovered buffers are the same as original
printf(" check recovery of block {");
for (i = 0; i < nerrs; i++) {
printf(" %d", frag_err_list[i]);
if (memcmp(recover_outp[i], frag_ptrs[frag_err_list[i]], len)) {
printf(" Fail erasure recovery %d, frag %d\n", i, frag_err_list[i]);
return -1;
}
}
printf(" } done encode: Pass\n");

// Check that recovered buffers are the same as original for the update version
printf(" check recovery of block {");
for (i = 0; i < nerrs; i++) {
printf(" %d", frag_err_list[i]);
if (memcmp(recover_outp_encode_update[i], frag_ptrs[frag_err_list[i]], len)) {
printf(" Fail erasure recovery %d, frag %d\n", i, frag_err_list[i]);
return -1;
}
}

printf(" } done encode_update: Pass\n");
return 0;
}

/*
* Generate decode matrix from encode matrix and erasure list
*
*/

static int gf_gen_decode_matrix_simple(u8 * encode_matrix,
u8 * decode_matrix,
u8 * invert_matrix,
u8 * temp_matrix,
u8 * decode_index, u8 * frag_err_list, int nerrs, int k,
int m)
{
int i, j, p, r;
int nsrcerrs = 0;
u8 s, *b = temp_matrix;
u8 frag_in_err[MMAX];

memset(frag_in_err, 0, sizeof(frag_in_err));

// Order the fragments in erasure for easier sorting
for (i = 0; i < nerrs; i++) {
if (frag_err_list[i] < k)
nsrcerrs++;
frag_in_err[frag_err_list[i]] = 1;
}

// Construct b (matrix that encoded remaining frags) by removing erased rows
for (i = 0, r = 0; i < k; i++, r++) {
while (frag_in_err[r])
r++;
for (j = 0; j < k; j++)
b[k * i + j] = encode_matrix[k * r + j];
decode_index[i] = r;
}

// Invert matrix to get recovery matrix
if (gf_invert_matrix(b, invert_matrix, k) < 0)
return -1;

// Get decode matrix with only wanted recovery rows
for (i = 0; i < nerrs; i++) {
if (frag_err_list[i] < k) // A src err
for (j = 0; j < k; j++)
decode_matrix[k * i + j] =
invert_matrix[k * frag_err_list[i] + j];
}

// For non-src (parity) erasures need to multiply encode matrix * invert
for (p = 0; p < nerrs; p++) {
if (frag_err_list[p] >= k) { // A parity err
for (i = 0; i < k; i++) {
s = 0;
for (j = 0; j < k; j++)
s ^= gf_mul(invert_matrix[j * k + i],
encode_matrix[k * frag_err_list[p] + j]);
decode_matrix[k * p + i] = s;
}
}
}
return 0;
}

```

If you change the line `recover_outp_encode_update[i][j] = rand();` to

`recover_outp_encode_update[i][j] = 0;`

Then the test passes without any error.

This shows that `ec_encode_data` will return the correct result even if you pass it an output buffer filled with random data, while `ec_encode_data_update` will not return the correct result if you pass it an output buffer filled with random data.

Am I doing something wrong?

Thanks!!

EDIT: Actually, thinking about it...it should be obvious that this is the case...how else would it work?

Of course it gives the wrong result when the output buffer is filled with random data, since you are supposed to run it iteratively, feeding in the result of the previous call to ec_encode_data_update. So if you mess up the result from one run, the next run will also give wrong results...

But still...I feel like this kind of thing should be in the docs, since it took me a few hours to figure out that the bug was due to the output buffers not being zero-initialized...is it so obvious that it doesn't need to be stated in the docs? maybe I am just too dumb to be the target audience for this library...