Add assertions to vm_reserv_test_config that are enforced by its caller. Change the type of variables 'hi' and 'lo', and change the a comparison to be in terms of pages, not bytes, to convince, perhaps, Coverity that nothing is amiss.
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
vm_reserv.c | ||
---|---|---|
1238 | If 'lo' and 'hi' stayed as 'int', coverity would still complain due to 'npages' being u_long, but since lo and hi are vm_paddr_t, it won't complain. I do think that 'lo + npages <= hi' is actually more readable. You might consider leaving 'lo' and 'hi' as 'int' (or even 'u_int') and changing 'npages' to 'u_int' instead of u_long. It would seem that 'npages' is confined to the range (0, VM_LEVEL_0_NPAGES]. |
vm_reserv.c | ||
---|---|---|
1174 | This comment is no longer correct. The requirement is that the reservation is locked. See the below assertion. | |
1195–1196 | I question the "+ PAGE_MASK" here. In other words, I suspect that "high" should be truncated, not rounded. | |
1224–1225 | An absurdly large alignment could cause integer overflow here as well. | |
1232–1233 | An absurdly large boundary could cause integer overflow here as well. | |
1266 | Since this function's return type is still boolean_t, the spelling of false should have remained "FALSE". |
Change bool literals to boolean_t literals in some places. Change u_ints to u_longs, and add a check for overflow. Truncate in computing hi.
vm_reserv.c | ||
---|---|---|
1256–1257 | Not here. Only in vm_reserv_test_contig(). This function acquires the reservation lock for vm_reserv_test_contig(). (And, to be clear, the original comment here "The free page queue lock must be held." is no longer correct and can be removed.) |
vm_reserv.c | ||
---|---|---|
1181 | There exists at least one platform, i386 PAE, where vm_paddr_t is wider than u_long, so simply changing "hi" and "lo" to u_long won't address the hypothetical overflow problems. (In practice, there would never be a problem because i386 PAE won't use all the bits available in vm_paddr_t, but a static analysis tool, like Coverity, is not going to know that.) | |
1184 | I would also assert that "npages < VM_LEVEL_0_NPAGES". |
Add an assertion. Use vm_paddr_t variable to manage bounds checking at function start.
vm_reserv.c | ||
---|---|---|
1181 | I believe that 32bit powerpc also got a similar mode recently. |
The code this modifies has been substantially changed now, and the benefits of this change no longer apply.