From: Jakub Witowski <[email protected]>
Contrary to the comment, implementation used a 8-byte MIC even if this
generated an additional segment.
---
mesh/model.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/mesh/model.c b/mesh/model.c
index a632d80e1..0b3ea4094 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -40,6 +40,8 @@
#include "mesh/util.h"
#include "mesh/model.h"
+#define CEILDIV(val, div) (((val) / (div)) + ((val) % (div) != 0))
+
struct mesh_model {
const struct mesh_model_ops *cbs;
void *user_data;
@@ -451,7 +453,7 @@ static bool msg_send(struct mesh_node *node, bool credential, uint16_t src,
/* 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);
}
--
2.19.1
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))
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