redundant compare

[louka]

Good evening,

I noticed while compiling ... has a redundant "limit >= 0". Is this worthy of a bug report?

(limit is an unsigned long long)

in vm_pageout.c (vm_daemon) line 1721

vm_pindex_t limit;

if (limit >= 0 && size >= limit) {
   vm_pageout_map_deactivate_pages(&vm->vm_map, limit);
}

 

[SirDice]

I noticed while compiling ... has a redundant "limit >= 0". Is this worthy of a bug report?

(limit is an unsigned long long)

You assume it's an unsigned long. For better security it doesn't hurt to double check.

 

[kpa]

The variable limit is in fact of type __uint64_t so the check is really redundant. This might be a leftover from an earlier version of the same code where limit was a signed type, probably a simple int.

 

[SirDice]

The function that's called (vm_pageout_map_deactivate_pages) accepts a long as a parameter. This means it would accept negative numbers.

 

[kpa]

Yes but still the variable limit is of an unsigned type and the first comparison limit >= 0 is always true. A variable of unsigned type will not magically change to signed just because the variable is later used in a signed context.

 

[xibo]

That condition is in fact a tautology since the type of limit cannot store any value that doesnot evaluate that expression to true. If vm_pageout_map_deactivate_pages requires it's parameter to be non-negative, it should either check it itself, or the condition should be either of a comparisation against LONGLONG_MAX (note the lack of U at the beginning) or a check for the carry bit.

Btw. that "condition" is never checked for at runtime since it is evaluated at compile time instead, so removing this redundant check would not affect anything. It would however impose an additional point to check for if the type of limit was to be changed in the future.