On Sat, 2009-11-14 at 19:59 -0700, David Brownell wrote: > On Saturday 14 November 2009, Zach Welch wrote: > > On Sat, 2009-11-14 at 12:56 -0700, David Brownell wrote: > > > On Saturday 14 November 2009, Zachary T Welch wrote: > > > > - for (unsigned i = 0, num_bytes = CEIL(size, 8); i < num_bytes; > > > > i++) > > > > - to[i] = from[i]; > > > > + // copy entire buffer > > > > + memcpy(_to, from, CEIL(size, 8)); > > > > > > Comment is wrong: copies all but trailing partial byte. > > > > CEIL is a strange name for converting a bit count to byte count. > > It's a rounding thing; "8" is what makes it perform such > a conversion. > > I'd be happier if it were called DIV_ROUND_UP() like a > certain OS kernel I could mention ... not as cryptic.
I agree that the current name stands to be improved. > (Likewise, there's a DIM which is used for ARRAY_SIZE. > And lots of re-inventions of that, since DIM is a > really un-obvious name for "what size is this array".) Again, I agree. I would be happy to handle these, but feel free to provide patches yourself them yourselves. Incidentally, I would like to see this later macro go into types.h, which is also where I suggest putting the container_of macro that is being used in the target layer. > > From > > src/helper/binarybuffer.h: > > > > #define CEIL(m, n) (((m) + (n) - 1) / (n)) > > > > Or is my brain not interpreting that code correctly? > > Or maybe I was. Given that it copies everything, why > is there code *after* the copy mangling the last byte? > Wouldn't that only be correct when the code copied > all but the trailing partial byte? It masks the last byte, ensuring any trailing bits are 0. Without it, the bits after the 'trailing' will have been copied beyond the bits intended: copy buf=0xff, len=3 should give 0x07 and not 0xff. Right? That's what the old code did, in a really confusing manner. > Looks to me like this code has always been trashing > that last byte. So "size/8" would be correct ... not > rounding up, just copy the whole bytes, then masking > in the trailing bits. I think my patch provides functional equivalence, but I may be wrong. --Z _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
