Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Gix, Brian" <brian.gix@intel.com>
To:     Michal Lowas-Rzechonek <jakub.witowski@silvair.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
CC:     "Stotland, Inga" <inga.stotland@intel.com>,
        "jakub.witowski@silvair.com" <jakub.witowski@silvair.com>
Subject: RE: [PATCH] mesh: Fixed log MIC usage in segmented messages
Thread-Topic: [PATCH] mesh: Fixed log MIC usage in segmented messages
Thread-Index: AQHVCUxamMxUmrxa9kagkMwDtpes56ZpOA0g
Date:   Mon, 13 May 2019 16:25:32 +0000
Message-ID: <DEBB0CAA2616974FAE35E4B560B9A4376CB4EE08@ORSMSX103.amr.corp.intel.com>
References: <20190507072702.27144-1-michal.lowas-rzechonek@silvair.com>
In-Reply-To: <20190507072702.27144-1-michal.lowas-rzechonek@silvair.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.0.600.7
dlp-reaction: no-action
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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))