Patchwork D9901: persistent-nodemap: Fix Rust declarations for Revlog_CAPI signatures

login
register
mail settings
Submitter phabricator
Date Jan. 28, 2021, 12:41 p.m.
Message ID <differential-rev-PHID-DREV-r66hz5zbj5clonfrbfne-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48212/
State Superseded
Headers show

Comments

phabricator - Jan. 28, 2021, 12:41 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Use Rust’s `libc::ssize_t` as the closest match to C’s `Py_ssize_t`.
  See details in test comment.
  
  Going forward we should find a way to have such Rust declarations
  auto-generated from C headers at build time,
  or auto-checked against them in a test.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

REVISION DETAIL
  https://phab.mercurial-scm.org/D9901

AFFECTED FILES
  rust/hg-cpython/src/cindex.rs
  tests/test-persistent-nodemap.t

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -33,10 +33,17 @@ 
 
 #if rust
 
-Reported bug: some Rust code panics when handling the null revision
+Regression test for a previous bug in Rust/C FFI for the `Revlog_CAPI` capsule:
+in places where C function signature use `Py_ssize_t` (64-bit on Linux x86_64),
+corresponding Rust declarations incorrectly used `libc::c_int` (32-bit).
+As a result, -1 passed from Rust for the null revision became 4294967295 in C.
 
-  $ hg log -r 00000000 2>&1 | grep panicked
-  thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', hg-cpython/src/revlog.rs:* (glob)
+  $ hg log -r 00000000
+  changeset:   -1:000000000000
+  tag:         tip
+  user:        
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  
 
 #endif
 
diff --git a/rust/hg-cpython/src/cindex.rs b/rust/hg-cpython/src/cindex.rs
--- a/rust/hg-cpython/src/cindex.rs
+++ b/rust/hg-cpython/src/cindex.rs
@@ -16,7 +16,7 @@ 
 };
 use hg::revlog::{Node, RevlogIndex};
 use hg::{Graph, GraphError, Revision, WORKING_DIRECTORY_REVISION};
-use libc::c_int;
+use libc::{c_int, ssize_t};
 
 const REVLOG_CABI_VERSION: c_int = 2;
 
@@ -24,10 +24,10 @@ 
 pub struct Revlog_CAPI {
     abi_version: c_int,
     index_length:
-        unsafe extern "C" fn(index: *mut revlog_capi::RawPyObject) -> c_int,
+        unsafe extern "C" fn(index: *mut revlog_capi::RawPyObject) -> ssize_t,
     index_node: unsafe extern "C" fn(
         index: *mut revlog_capi::RawPyObject,
-        rev: c_int,
+        rev: ssize_t,
     ) -> *const Node,
     index_parents: unsafe extern "C" fn(
         index: *mut revlog_capi::RawPyObject,
@@ -157,7 +157,7 @@ 
 
     fn node(&self, rev: Revision) -> Option<&Node> {
         let raw = unsafe {
-            (self.capi.index_node)(self.index.as_ptr(), rev as c_int)
+            (self.capi.index_node)(self.index.as_ptr(), rev as ssize_t)
         };
         if raw.is_null() {
             None