#6334 closed defect (fixed)
Suspicious asserts in cpl_virtualmem.cpp
Reported by: | Kurt Schwehr | Owned by: | Even Rouault |
---|---|---|---|
Priority: | normal | Milestone: | 2.1.0 |
Component: | default | Version: | unspecified |
Severity: | critical | Keywords: | |
Cc: |
Description
The munmap never happens in optimized mode?!?!
assert(munmap(ctxt->sBase.pDataToFree, nRoundedMappingSize) == 0);
And for mprotect, is it okay to not call it?
assert(mprotect(pPageToFill, ctxt->sBase.nPageSize, PROT_READ | PROT_WRITE) == 0);
Should the write really be wrapped in an assert? This means that the write never happens in optimized mode.
/* Pass the address that caused the fault to the helper thread */ assert(write(pVirtualMemManager->pipefd_to_thread[1], msg, sizeof(*msg)) == sizeof(*msg));
e.g. on Ubuntu 14.04 LTS, I see assert defined like this:
#ifdef NDEBUG # define assert(expr) (__ASSERT_VOID_CAST (0))
So the code inside an assert just goes away. If it is okay for that to happen, it would be helpful to have an explanation why it is okay for all the function / method calls inside of asserts to be done that way.
Change History (2)
comment:1 by , 8 years ago
Milestone: | → 2.1.0 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:2 by , 8 years ago
I also build GDAL using http://bazel.io/ with a totally custom environment.
Note:
See TracTickets
for help on using tickets.
Good catch (how did you found this ?). GDAL builds do not normally define NDEBUG, but it is not sane to leave side effects in assert()
I've move away the side effects and test the return value with the following IgnoreOrAssertInDebug()
trunk r33168 "port/cpl_virtualmem.cpp: don't run code with side effects in assert() in case NDEBUG is defined (#6334)"