test/mremap.c (1)

1-24: Test logic is sound; consider small robustness/portability tweaks

The loop correctly exercises mremap with MREMAP_MAYMOVE | MREMAP_FIXED and verifies that the mapping ends up at old_addr2, which should catch the wrapper bug.

Two non‑blocking suggestions to make the test more robust:

• Use runtime page size instead of hard‑coding 4096. On systems with a different page size, munmap(old_addr2, 4096) can fail and leave old_addr2 still mapped, making the mremap precondition false. Using getpagesize() (already available via <unistd.h>) avoids this:

  int main() {
      size_t page_size = (size_t)getpagesize();
      void *old_addr1, *old_addr2, *new_addr;
      for (;;) {
          old_addr1 = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
                           MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
          assert(old_addr1 != MAP_FAILED);
  
          old_addr2 = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
                           MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
          assert(old_addr2 != MAP_FAILED);
  
          assert(munmap(old_addr2, page_size) == 0);
          new_addr = mremap(old_addr1, page_size, page_size,
                            MREMAP_MAYMOVE | MREMAP_FIXED, old_addr2);
          assert(new_addr == old_addr2);
          assert(munmap(old_addr2, page_size) == 0);
          sleep(1);
      }
  }

• Add basic mmap/munmap error checks. As above, simple assert checks give clearer failures if the environment is misconfigured, rather than letting later calls behave unexpectedly.

If you want to keep the test minimal, even just switching to getpagesize() would address the main portability concern.

src/mtcp/mtcp_restart.c (1)

971-995: Comment update above mremap_move is accurate; optional spelling nit

The added note that MREMAP_MAYMOVE is required when using MREMAP_FIXED is correct and aligns with how mremap_move invokes mtcp_sys_mremap.

Minor nit (optional): in the comment, “desgination” could be corrected to “destination” for clarity:

// the memory was moved to the wrong destination.  Undo the move, and return -1.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

test/mremap.c (2)

11-15: Consider adding error handling for mmap and munmap calls.

While test programs may have relaxed error handling, adding checks for MAP_FAILED on mmap calls and return value checks on munmap would make test failures more explicit and easier to debug.

Example:

     old_addr1 = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    assert(old_addr1 != MAP_FAILED);
     old_addr2 = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    assert(old_addr2 != MAP_FAILED);
-    munmap(old_addr2, PAGE_SIZE);
+    assert(munmap(old_addr2, PAGE_SIZE) == 0);

---

20-21: Consider checking munmap return value for consistency.

Similar to Line 15, checking the return value of this munmap call would maintain consistency with error handling suggestions.

-    munmap(old_addr2, PAGE_SIZE);
+    assert(munmap(old_addr2, PAGE_SIZE) == 0);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro