From: Zooko O'Whielacronx Date: Thu, 1 Feb 2007 07:03:25 +0000 (-0700) Subject: pyfec: fix preconditions and typing, remove unused error-checking, tidy-up naming... X-Git-Tag: tahoe_v0.1.0-0-UNSTABLE~283 X-Git-Url: https://git.rkrishnan.org/pf/content/en/seg/frontends/?a=commitdiff_plain;h=8f17174cb8c74ea8e0bcf28913aca8359bdce75e;p=tahoe-lafs%2Ftahoe-lafs.git pyfec: fix preconditions and typing, remove unused error-checking, tidy-up naming and documentation --- diff --git a/pyfec/fec/_fecmodule.c b/pyfec/fec/_fecmodule.c index 8bd6f46c..97a935e4 100644 --- a/pyfec/fec/_fecmodule.c +++ b/pyfec/fec/_fecmodule.c @@ -111,8 +111,8 @@ Encoder_init(Encoder *self, PyObject *args, PyObject *kwdict) { py_raise_fec_error("Precondition violation: second argument is required to be greater than or equal to 1, but it was %d", self->mm); return -1; } - if (self->mm > 255) { - py_raise_fec_error("Precondition violation: second argument is required to be less than or equal to 255, but it was %d", self->mm); + if (self->mm > 256) { + py_raise_fec_error("Precondition violation: second argument is required to be less than or equal to 256, but it was %d", self->mm); return -1; } if (self->kk > self->mm) { @@ -142,14 +142,14 @@ Encoder_encode(Encoder *self, PyObject *args) { gf* check_shares_produced[self->mm - self->kk]; /* This is an upper bound -- we will actually use only num_check_shares_produced of these elements (see below). */ PyObject* pystrs_produced[self->mm - self->kk]; /* This is an upper bound -- we will actually use only num_check_shares_produced of these elements (see below). */ - unsigned char num_check_shares_produced = 0; /* The first num_check_shares_produced elements of the check_shares_produced array and of the pystrs_produced array will be used. */ + unsigned num_check_shares_produced = 0; /* The first num_check_shares_produced elements of the check_shares_produced array and of the pystrs_produced array will be used. */ const gf* incshares[self->kk]; - unsigned char num_desired_shares; + unsigned num_desired_shares; PyObject* fast_desired_shares_ids = NULL; PyObject** fast_desired_shares_ids_items; - unsigned char c_desired_shares_ids[self->mm]; - unsigned char c_desired_checkshares_ids[self->mm - self->kk]; - unsigned char i; + unsigned c_desired_shares_ids[self->mm]; + unsigned c_desired_checkshares_ids[self->mm - self->kk]; + unsigned i; if (desired_shares_ids) { fast_desired_shares_ids = PySequence_Fast(desired_shares_ids, "Second argument (optional) was not a sequence."); num_desired_shares = PySequence_Fast_GET_SIZE(fast_desired_shares_ids); @@ -360,8 +360,8 @@ Decoder_init(Encoder *self, PyObject *args, PyObject *kwdict) { py_raise_fec_error("Precondition violation: second argument is required to be greater than or equal to 1, but it was %d", self->mm); return -1; } - if (self->mm > 255) { - py_raise_fec_error("Precondition violation: second argument is required to be less than or equal to 255, but it was %d", self->mm); + if (self->mm > 256) { + py_raise_fec_error("Precondition violation: second argument is required to be less than or equal to 256, but it was %d", self->mm); return -1; } if (self->kk > self->mm) { @@ -393,10 +393,10 @@ Decoder_decode(Decoder *self, PyObject *args) { return NULL; const gf*restrict cshares[self->kk]; - unsigned char cshareids[self->kk]; + unsigned cshareids[self->kk]; gf*restrict recoveredcstrs[self->kk]; /* self->kk is actually an upper bound -- we probably won't need all of this space. */ PyObject*restrict recoveredpystrs[self->kk]; /* self->kk is actually an upper bound -- we probably won't need all of this space. */ - unsigned char i; + unsigned i; for (i=0; ikk; i++) recoveredpystrs[i] = NULL; PyObject*restrict fastshares = PySequence_Fast(shares, "First argument was not a sequence."); @@ -416,7 +416,7 @@ Decoder_decode(Decoder *self, PyObject *args) { } /* Construct a C array of gf*'s of the data and another of C ints of the shareids. */ - unsigned char needtorecover=0; + unsigned needtorecover=0; PyObject** fastshareidsitems = PySequence_Fast_ITEMS(fastshareids); if (!fastshareidsitems) goto err; @@ -428,11 +428,11 @@ Decoder_decode(Decoder *self, PyObject *args) { if (!PyInt_Check(fastshareidsitems[i])) goto err; long tmpl = PyInt_AsLong(fastshareidsitems[i]); - if (tmpl < 0 || tmpl >= UCHAR_MAX) { + if (tmpl < 0 || tmpl > 255) { py_raise_fec_error("Precondition violation: Share ids can't be less than zero or greater than 255. %ld\n", tmpl); goto err; } - cshareids[i] = (unsigned char)tmpl; + cshareids[i] = (unsigned)tmpl; if (cshareids[i] >= self->kk) needtorecover+=1; @@ -455,7 +455,7 @@ Decoder_decode(Decoder *self, PyObject *args) { i++; else { /* put pkt in the right position. */ - unsigned char c = cshareids[i]; + unsigned c = cshareids[i]; SWAP (cshareids[i], cshareids[c], int); SWAP (cshares[i], cshares[c], const gf*); @@ -477,7 +477,7 @@ Decoder_decode(Decoder *self, PyObject *args) { fec_decode(self->fec_matrix, cshares, recoveredcstrs, cshareids, sz); /* Wrap up both original primary shares and decoded shares into a Python list of Python strings. */ - unsigned char nextrecoveredix=0; + unsigned nextrecoveredix=0; result = PyList_New(self->kk); if (result == NULL) goto err; diff --git a/pyfec/fec/fec.c b/pyfec/fec/fec.c index ba43da95..f563b861 100644 --- a/pyfec/fec/fec.c +++ b/pyfec/fec/fec.c @@ -128,7 +128,7 @@ modnn(int x) { * many numbers by the same constant. In this case the first call sets the * constant, and others perform the multiplications. A value related to the * multiplication is held in a local variable declared with USE_GF_MULC . See - * usage in addmul1(). + * usage in _addmul1(). */ static gf gf_mul_table[256][256]; @@ -174,7 +174,7 @@ my_malloc (int sz, char *err_string) { } #define NEW_GF_MATRIX(rows, cols) \ - (gf *)my_malloc(rows * cols * sizeof(gf), " ## __LINE__ ## " ) + (gf*)my_malloc(rows * cols, " ## __LINE__ ## " ) /* * initialize the data structures used for computations in GF. @@ -252,15 +252,13 @@ generate_gf (void) { * calls are unfrequent in my typical apps so I did not bother. */ #define addmul(dst, src, c, sz) \ - if (c != 0) addmul1(dst, src, c, sz) + if (c != 0) _addmul1(dst, src, c, sz) #define UNROLL 16 /* 1, 4, 8, 16 */ static void -addmul1 (gf * dst1, const gf * src1, gf c, int sz) { +_addmul1(register gf*restrict dst, const register gf*restrict src, gf c, size_t sz) { USE_GF_MULC; - register gf *dst = dst1; - register const gf *src = src1; - gf *lim = &dst[sz - UNROLL + 1]; + const gf* lim = &dst[sz - UNROLL + 1]; GF_MULC0 (c); @@ -297,8 +295,8 @@ addmul1 (gf * dst1, const gf * src1, gf c, int sz) { * computes C = AB where A is n*k, B is k*m, C is n*m */ static void -matmul (gf * a, gf * b, gf * c, int n, int k, int m) { - int row, col, i; +_matmul(gf * a, gf * b, gf * c, unsigned n, unsigned k, unsigned m) { + unsigned row, col, i; for (row = 0; row < n; row++) { for (col = 0; col < m; col++) { @@ -313,20 +311,21 @@ matmul (gf * a, gf * b, gf * c, int n, int k, int m) { } /* - * invert_mat() takes a matrix and produces its inverse + * _invert_mat() takes a matrix and produces its inverse * k is the size of the matrix. * (Gauss-Jordan, adapted from Numerical Recipes in C) * Return non-zero if singular. */ -static int -invert_mat (gf * src, int k) { +static void +_invert_mat(gf* src, unsigned k) { gf c, *p; - int irow, icol, row, col, i, ix; + unsigned irow = 0; + unsigned icol = 0; + unsigned row, col, i, ix; - int error = -1; - int *indxc = (int *) my_malloc (k * sizeof (int), "indxc"); - int *indxr = (int *) my_malloc (k * sizeof (int), "indxr"); - int *ipiv = (int *) my_malloc (k * sizeof (int), "ipiv"); + unsigned* indxc = (unsigned*) my_malloc (k * sizeof(unsigned), "indxc"); + unsigned* indxr = (unsigned*) my_malloc (k * sizeof(unsigned), "indxr"); + unsigned* ipiv = (unsigned*) my_malloc (k * sizeof(unsigned), "ipiv"); gf *id_row = NEW_GF_MATRIX (1, k); gf *temp_row = NEW_GF_MATRIX (1, k); @@ -343,7 +342,6 @@ invert_mat (gf * src, int k) { * Zeroing column 'col', look for a non-zero element. * First try on the diagonal, if it fails, look elsewhere. */ - irow = icol = -1; if (ipiv[col] != 1 && src[col * k + col] != 0) { irow = col; icol = col; @@ -365,10 +363,6 @@ invert_mat (gf * src, int k) { } } } - if (icol == -1) { - ERR("Pivot not found!"); - goto fail; - } found_piv: ++(ipiv[icol]); /* @@ -416,26 +410,17 @@ invert_mat (gf * src, int k) { } id_row[icol] = 0; } /* done all columns */ - for (col = k - 1; col >= 0; col--) { - if (indxr[col] < 0 || indxr[col] >= k) { - ERR("AARGH, indxr[col] %d\n", indxr[col]); - goto fail; - } else if (indxc[col] < 0 || indxc[col] >= k) { - ERR("AARGH, indxc[col] %d\n", indxc[col]); - goto fail; - } else if (indxr[col] != indxc[col]) { + for (col = k; col > 0; col--) + if (indxr[col-1] != indxc[col-1]) for (row = 0; row < k; row++) - SWAP (src[row * k + indxr[col]], src[row * k + indxc[col]], gf); - } - } - error = 0; + SWAP (src[row * k + indxr[col-1]], src[row * k + indxc[col-1]], gf); fail: free (indxc); free (indxr); free (ipiv); free (id_row); free (temp_row); - return error; + return; } /* @@ -449,14 +434,14 @@ invert_mat (gf * src, int k) { * p = coefficients of the matrix (p_i) * q = values of the polynomial (known) */ -int -invert_vdm (gf * src, int k) { - int i, j, row, col; +void +_invert_vdm (gf* src, unsigned k) { + unsigned i, j, row, col; gf *b, *c, *p; gf t, xx; if (k == 1) /* degenerate case, matrix must be p^0 = 1 */ - return 0; + return; /* * c holds the coefficient of P(x) = Prod (x - p_i), i=0..k-1 * b holds the coefficient for the matrix inversion @@ -491,9 +476,9 @@ invert_vdm (gf * src, int k) { xx = p[row]; t = 1; b[k - 1] = 1; /* this is in fact c[k] */ - for (i = k - 2; i >= 0; i--) { - b[i] = c[i + 1] ^ gf_mul (xx, b[i + 1]); - t = gf_mul (xx, t) ^ b[i]; + for (i = k - 1; i > 0; i--) { + b[i-1] = c[i] ^ gf_mul (xx, b[i]); + t = gf_mul (xx, t) ^ b[i-1]; } for (col = 0; col < k; col++) src[col * k + row] = gf_mul (inverse[t], b[col]); @@ -501,7 +486,7 @@ invert_vdm (gf * src, int k) { free (c); free (b); free (p); - return 0; + return; } static int fec_initialized = 0; @@ -531,13 +516,9 @@ fec_free (fec_t *p) { free (p); } -/* - * create a new encoder, returning a descriptor. This contains k,n and - * the encoding matrix. - */ fec_t * -fec_new (unsigned char k, unsigned char n) { - unsigned char row, col; +fec_new(unsigned k, unsigned n) { + unsigned row, col; gf *p, *tmp_m; fec_t *retval; @@ -569,8 +550,8 @@ fec_new (unsigned char k, unsigned char n) { * k*k vandermonde matrix, multiply right the bottom n-k rows * by the inverse, and construct the identity matrix at the top. */ - invert_vdm (tmp_m, k); /* much faster than invert_mat */ - matmul (tmp_m + k * k, tmp_m, retval->enc_matrix + k * k, n - k, k, k); + _invert_vdm (tmp_m, k); /* much faster than _invert_mat */ + _matmul(tmp_m + k * k, tmp_m, retval->enc_matrix + k * k, n - k, k, k); /* * the upper matrix is I so do not bother with a slow multiply */ @@ -583,9 +564,9 @@ fec_new (unsigned char k, unsigned char n) { } void -fec_encode(const fec_t* code, const gf*restrict const*restrict const src, gf*restrict const*restrict const fecs, const unsigned char*restrict const share_ids, unsigned char num_share_ids, size_t sz) { +fec_encode(const fec_t* code, const gf*restrict const*restrict const src, gf*restrict const*restrict const fecs, const unsigned*restrict const share_ids, size_t num_share_ids, size_t sz) { unsigned i, j; - unsigned char fecnum; + unsigned fecnum; gf* p; for (i=0; ienc_matrix[fecnum * code->k]); for (j = 0; j < code->k; j++) - addmul (fecs[i], src[j], p[j], sz); + addmul(fecs[i], src[j], p[j], sz); } } @@ -604,7 +585,7 @@ fec_encode(const fec_t* code, const gf*restrict const*restrict const src, gf*res * @param matrix a space allocated for a k by k matrix */ void -build_decode_matrix_into_space(const fec_t*restrict const code, const unsigned char*const restrict index, const unsigned char k, gf*restrict const matrix) { +build_decode_matrix_into_space(const fec_t*restrict const code, const unsigned*const restrict index, const unsigned k, gf*restrict const matrix) { unsigned i; gf* p; for (i=0, p=matrix; i < k; i++, p += k) { @@ -615,11 +596,11 @@ build_decode_matrix_into_space(const fec_t*restrict const code, const unsigned c memcpy(p, &(code->enc_matrix[index[i] * code->k]), k); } } - invert_mat (matrix, k); + _invert_mat (matrix, k); } void -fec_decode(const fec_t* code, const gf*restrict const*restrict const inpkts, gf*restrict const*restrict const outpkts, const unsigned char*restrict const index, size_t sz) { +fec_decode(const fec_t* code, const gf*restrict const*restrict const inpkts, gf*restrict const*restrict const outpkts, const unsigned*restrict const index, size_t sz) { gf m_dec[code->k * code->k]; build_decode_matrix_into_space(code, index, code->k, m_dec); diff --git a/pyfec/fec/fec.h b/pyfec/fec/fec.h index 038b2c14..abce4688 100644 --- a/pyfec/fec/fec.h +++ b/pyfec/fec/fec.h @@ -71,12 +71,16 @@ typedef unsigned char gf; typedef struct { unsigned long magic; - unsigned char k, n; /* parameters of the code */ + unsigned k, n; /* parameters of the code */ gf* enc_matrix; } fec_t; -void fec_free (fec_t* p); -fec_t* fec_new (unsigned char k, unsigned char n); +/** + * param k the number of shares required to reconstruct + * param m the total number of share created + */ +fec_t* fec_new(unsigned k, unsigned m); +void fec_free(fec_t* p); /** * @param inpkts the "primary shares" i.e. the chunks of the input data @@ -84,7 +88,7 @@ fec_t* fec_new (unsigned char k, unsigned char n); * @param share_ids the numbers of the desired shares -- including both primary shares (the id < k) which fec_encode() ignores and check shares (the id >= k) which fec_encode() will produce and store into the buffers of the fecs parameter * @param num_share_ids the length of the share_ids array */ -void fec_encode(const fec_t* code, const gf*restrict const*restrict const src, gf*restrict const*restrict const fecs, const unsigned char*restrict const share_ids, unsigned char num_share_ids, size_t sz); +void fec_encode(const fec_t* code, const gf*restrict const*restrict const src, gf*restrict const*restrict const fecs, const unsigned*restrict const share_ids, size_t num_share_ids, size_t sz); /** * @param inpkts an array of packets (size k) @@ -92,6 +96,6 @@ void fec_encode(const fec_t* code, const gf*restrict const*restrict const src, g * @param index an array of the shareids of the packets in inpkts * @param sz size of a packet in bytes */ -void fec_decode(const fec_t* code, const gf*restrict const*restrict const inpkts, gf*restrict const*restrict const outpkts, const unsigned char*restrict const index, size_t sz); +void fec_decode(const fec_t* code, const gf*restrict const*restrict const inpkts, gf*restrict const*restrict const outpkts, const unsigned*restrict const index, size_t sz); /* end of file */ diff --git a/pyfec/fec/test/test_pyfec.py b/pyfec/fec/test/test_pyfec.py index 16579c88..fc5f8fd1 100644 --- a/pyfec/fec/test/test_pyfec.py +++ b/pyfec/fec/test/test_pyfec.py @@ -30,6 +30,17 @@ import sys import fec +from base64 import b32encode +def ab(x): # debuggery + if len(x) >= 3: + return "%s:%s" % (len(x), b32encode(x[-3:]),) + elif len(x) == 2: + return "%s:%s" % (len(x), b32encode(x[-2:]),) + elif len(x) == 1: + return "%s:%s" % (len(x), b32encode(x[-1:]),) + elif len(x) == 0: + return "%s:%s" % (len(x), "--empty--",) + def _h(k, m, ss): # sys.stdout.write("k: %s, m: %s, len(ss): %r, len(ss[0]): %r" % (k, m, len(ss), len(ss[0]),)) ; sys.stdout.flush() encer = fec.Encoder(k, m) @@ -47,7 +58,7 @@ def _h(k, m, ss): decoded = decer.decode(shares, nums) # sys.stdout.write("decoded.\n") ; sys.stdout.flush() assert len(decoded) == len(ss), (len(decoded), len(ss),) - assert tuple([str(s) for s in decoded]) == tuple([str(s) for s in ss]), (tuple([str(s) for s in decoded]), tuple([str(s) for s in ss]),) + assert tuple([str(s) for s in decoded]) == tuple([str(s) for s in ss]), (tuple([ab(str(s)) for s in decoded]), tuple([ab(str(s)) for s in ss]),) def randstr(n): return ''.join(map(chr, map(random.randrange, [0]*n, [256]*n))) @@ -81,7 +92,7 @@ def _test_random(): _h(k, m, ss) def test_random(): - for i in range(2**5): + for i in range(2**7): # sys.stdout.write(",") _test_random() # sys.stdout.write(".")