[sheepdog] [PATCH 1/9] add forward error correction for erasure code

Liu Yuan namei.unix at gmail.com
Tue Sep 24 14:30:25 CEST 2013


On Tue, Sep 24, 2013 at 05:14:14PM +0900, MORITA Kazutaka wrote:
> At Fri, 20 Sep 2013 17:11:34 +0900,
> Hitoshi Mitake wrote:
> > 
> > 
> > This would be a great contribution!
> > 
> > I'd like to post some trivial comments not related to essential part
> > of erasure coding.
> > 
> > At Thu, 19 Sep 2013 18:42:45 +0800,
> > Liu Yuan wrote:
> > > +#include <stddef.h>
> > > +#include <stdint.h>
> > > +
> > > +#ifdef __GNUC__
> > > +#ifndef alloca
> > > +#define alloca(x) __builtin_alloca(x)
> > > +#endif
> > > +#else
> > > +#include <alloca.h>
> > > +#endif
> > 
> > This alloca() is only used by fec_decode() in lib/fec.c. The caller
> > part,
> > 	uint8_t *m_dec = (uint8_t *)alloca(code->k * code->k);
> > can be replaced by this because we are using C99 style:
> > 	uint8_t m_dec[code->k * code->k];
> > 
> > Let's eliminate the alloca() definition.
> 
> I don't like a variable length array, either.  Those have a risk of
> stack overflow.
> 
> Can we limit the max size of the array?  Then we can replace it with like
> 
>   uint8_t m_dec[MAX_DECODE_BUF];
> 
> Or, please add at least assert() to detect too big memory allocation.

Add a assert() looks good enough, normally k is just a very small integer.

Thanks
Yuan



More information about the sheepdog mailing list