Re: [PATCH] mesh: Fixed log MIC usage in segmented messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux