Hi Michal & Jakub, On Mon, 2019-05-13 at 09:25 -0700, Gix, Brian wrote: > Hi Michal & Jakub, > > > From: Michal Lowas-Rzechonek > > +#define CEILDIV(val, div) (((val) / (div)) + ((val) % (div) != 0)) > + > > > > /* Use large MIC if it doesn't affect segmentation */ > if (msg_len > 11 && msg_len <= 376) { > - if ((out_len / 12) == ((out_len + 4) / 12)) { > + if (CEILDIV(out_len, 12) == CEILDIV(out_len + 4, 12)) { > szmic = true; > out_len = msg_len + sizeof(uint64_t); > } > > I see what you are doing here, and agree that it will fix a > problem... for instance with out_len == 20 > > I am uncomfortable with two things: > > 1. The name CEILDIV... I found a definition for it (divide and > round up) but I think it perhaps either the macro should be renamed > something like "SEG_COUNT" *or* a comment added defining what > CEILDIV means for the uninitiated (like me): > > /* CEILDIV() is a Divide and Round Up macro */ > > > > 2. Using a Boolean result as an Integer for addition makes the macro > a bit difficult to puzzle out, if you don't know what it is trying to > do. > > What about: > (((val) / (div)) + (((val) % (div)) ? 1 : 0)) > > I believe the standard way of implementing "divide and round up to ceiling" is: (((val) + (div) - 1) / (div)) Side note: since in the particular context of where this macro is going to be called, the values of both val and div are positive and have been vetted not to exceed the upper limit of mesh payload length, there's no danger of an overflow and the macro does not require any additional checks. Best, Inga