Re: [RFC v2] Reed-Solomon Code: Update no_eras to the actual number of errors

[Aiden Leong]

Hi,

You are right. I forgot the return value is number of errors rather than 
status code.

Sorry to bother you.

On 6/25/20 6:06 AM, Ferdinand Blomqvist wrote:
> Hi!
>
> On 2020-06-25 00:36:01, Aiden Leong wrote:
> > Corr and eras_pos are updated to actual correction pattern and erasure
> > positions, but no_eras is not.
> >
> > When this library is used to recover lost bytes, we normally memset the
> > lost trunk of bytes to zero as a placeholder. Unfortunately, if the lost
> > byte is zero, b[i] is zero too. Without correct no_eras, users won't be
> > able to determine the valid length of corr and eras_pos.
> >
> > Signed-off-by: Aiden Leong <aiden.leong@xxxxxxxxx>
>
> I'm not sure I understand what you try to do. decode_rs* already returns
> the number of errors correted (or something negative upon failure). So
> your last statment is false. The lengt of corr and eras_pos is returned
> by the function. So this change is unnecessary. More comments inline.
>
> > diff --git a/lib/reed_solomon/decode_rs.c b/lib/reed_solomon/decode_rs.c
> > index 805de84ae83d..44136ea33d16 100644
> > --- a/lib/reed_solomon/decode_rs.c
> > +++ b/lib/reed_solomon/decode_rs.c
> > @@ -24,6 +24,7 @@
> > ÂÂÂÂint count = 0;
> > ÂÂÂÂint num_corrected;
> > ÂÂÂÂuint16_t msk = (uint16_t) rs->nn;
> > +ÂÂÂ int no_eras_local = no_eras ? *no_eras : 0;
> >
> > ÂÂÂÂ/*
> > ÂÂÂÂ * The decoder buffers are in the rs control struct. They are
> > @@ -106,11 +107,11 @@
> > ÂÂÂÂmemset(&lambda[1], 0, nroots * sizeof(lambda[0]));
> > ÂÂÂÂlambda[0] = 1;
> >
> > -ÂÂÂ if (no_eras > 0) {
> > +ÂÂÂ if (no_eras_local > 0) {
> > ÂÂÂÂÂÂÂ /* Init lambda to be the erasure locator polynomial */
> > ÂÂÂÂÂÂÂ lambda[1] = alpha_to[rs_modnn(rs,
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ prim * (nn - 1 - (eras_pos[0] + pad)))];
> > -ÂÂÂÂÂÂÂ for (i = 1; i < no_eras; i++) {
> > +ÂÂÂÂÂÂÂ for (i = 1; i < no_eras_local; i++) {
> > ÂÂÂÂÂÂÂÂÂÂÂ u = rs_modnn(rs, prim * (nn - 1 - (eras_pos[i] + pad)));
> > ÂÂÂÂÂÂÂÂÂÂÂ for (j = i + 1; j > 0; j--) {
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ tmp = index_of[lambda[j - 1]];
> > @@ -129,8 +130,8 @@
> > ÂÂÂÂ * Begin Berlekamp-Massey algorithm to determine error+erasure
> > ÂÂÂÂ * locator polynomial
> > ÂÂÂÂ */
> > -ÂÂÂ r = no_eras;
> > -ÂÂÂ el = no_eras;
> > +ÂÂÂ r = no_eras_local;
> > +ÂÂÂ el = no_eras_local;
> > ÂÂÂÂwhile (++r <= nroots) {ÂÂÂ /* r is the step number */
> > ÂÂÂÂÂÂÂ /* Compute discrepancy at the r-th step in poly-form */
> > ÂÂÂÂÂÂÂ discr_r = 0;
> > @@ -158,8 +159,8 @@
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ } else
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ t[i + 1] = lambda[i + 1];
> > ÂÂÂÂÂÂÂÂÂÂÂ }
> > -ÂÂÂÂÂÂÂÂÂÂÂ if (2 * el <= r + no_eras - 1) {
> > -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ el = r + no_eras - el;
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (2 * el <= r + no_eras_local - 1) {
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ el = r + no_eras_local - el;
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ /*
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ * 2 lines below: B(x) <-- inv(discr_r) *
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ * lambda(x)
> > @@ -312,14 +313,21 @@
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ eras_pos[j++] = loc[i] - pad;
> > ÂÂÂÂÂÂÂÂÂÂÂ }
> > ÂÂÂÂÂÂÂ }
> > +ÂÂÂÂÂÂÂ if (no_eras)
> > +ÂÂÂÂÂÂÂÂÂÂÂ *no_eras = j;
> At this point j will be equal to num_corrected. So why return this
> information in no_eras, when it is already returned by the function?
>
> > ÂÂÂÂ} else if (data && par) {
> > ÂÂÂÂÂÂÂ /* Apply error to data and parity */
> > +ÂÂÂÂÂÂÂ j = 0;
> > ÂÂÂÂÂÂÂ for (i = 0; i < count; i++) {
> > ÂÂÂÂÂÂÂÂÂÂÂ if (loc[i] < (nn - nroots))
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ data[loc[i] - pad] ^= b[i];
> > ÂÂÂÂÂÂÂÂÂÂÂ else
> > ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ par[loc[i] - pad - len] ^= b[i];
> > +ÂÂÂÂÂÂÂÂÂÂÂ if (b[i])
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ j++;
> > ÂÂÂÂÂÂÂ }
> > +ÂÂÂÂÂÂÂ if (no_eras)
> > +ÂÂÂÂÂÂÂÂÂÂÂ *no_eras = j;
>
> Same as above.
>
> > 2.25.1
>
> Best,
> Ferdinand