Skip to content

Commit ed0f49e

Browse files
committed
capi: swap *link arguments to match syscalls
For the pathrs_inroot_* API, it is a little cleaner to always have the path to resolve be the first argument. However, when writing the e2e tests I realised that diverging from the syscall API is just going to cause confusion. This is also a decent test case for our symbol versioning being used to keep old binaries working (though in practice this is just a straight-up breakage that doesn't need to be patched over). Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 80ca8d3 commit ed0f49e

File tree

7 files changed

+83
-32
lines changed

7 files changed

+83
-32
lines changed

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
66

77
## [Unreleased] ##
88

9+
### Breaking ###
10+
* `pathrs_inroot_hardlink` and `pathrs_inroot_symlink` have been switched to
11+
using the standard arguemnt order from their respective system calls
12+
(previously the order was swapped, which lead to possible confusion).
13+
- Previously compiled programs will continue to work but rebuilt programs
14+
will need to adjust their argument order.
15+
- Rust users are not affected by this change.
16+
- For the Go and Python bindings, the wrappers have also had their argument
17+
orders swapped to match the C API and so will also need to be updated when
18+
rebuilding.
19+
920
## [0.2.2] - 2025-11-25 ##
1021

1122
> 貴様ら全員刀の錆にしてやるぜ。

contrib/bindings/python/pathrs/_pathrs.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def mknod(self, path: str, mode: int, device: int = 0, /) -> None:
359359
if _is_pathrs_err(err):
360360
raise PathrsError._fetch(err) or INTERNAL_ERROR
361361

362-
def hardlink(self, path: str, target: str, /) -> None:
362+
def hardlink(self, target: str, linkname: str, /) -> None:
363363
"""
364364
Create a hardlink between two paths inside the Root.
365365
@@ -370,12 +370,12 @@ def hardlink(self, path: str, target: str, /) -> None:
370370
exists.
371371
"""
372372
err = libpathrs_so.pathrs_inroot_hardlink(
373-
self.fileno(), _cstr(path), _cstr(target)
373+
self.fileno(), _cstr(target), _cstr(linkname)
374374
)
375375
if _is_pathrs_err(err):
376376
raise PathrsError._fetch(err) or INTERNAL_ERROR
377377

378-
def symlink(self, path: str, target: str, /) -> None:
378+
def symlink(self, target: str, linkname: str, /) -> None:
379379
"""
380380
Create a symlink at the given path in the Root.
381381
@@ -387,7 +387,7 @@ def symlink(self, path: str, target: str, /) -> None:
387387
exists.
388388
"""
389389
err = libpathrs_so.pathrs_inroot_symlink(
390-
self.fileno(), _cstr(path), _cstr(target)
390+
self.fileno(), _cstr(target), _cstr(linkname)
391391
)
392392
if _is_pathrs_err(err):
393393
raise PathrsError._fetch(err) or INTERNAL_ERROR

go-pathrs/internal/libpathrs/libpathrs_linux.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,26 +193,26 @@ func InRootMknod(rootFd uintptr, path string, mode uint32, dev uint64) error {
193193
}
194194

195195
// InRootSymlink wraps pathrs_inroot_symlink.
196-
func InRootSymlink(rootFd uintptr, path, target string) error {
197-
cPath := C.CString(path)
198-
defer C.free(unsafe.Pointer(cPath))
196+
func InRootSymlink(rootFd uintptr, target, linkpath string) error {
197+
cLinkPath := C.CString(linkpath)
198+
defer C.free(unsafe.Pointer(cLinkPath))
199199

200200
cTarget := C.CString(target)
201201
defer C.free(unsafe.Pointer(cTarget))
202202

203-
err := C.pathrs_inroot_symlink(C.int(rootFd), cPath, cTarget)
203+
err := C.pathrs_inroot_symlink(C.int(rootFd), cLinkPath, cTarget)
204204
return fetchError(err)
205205
}
206206

207207
// InRootHardlink wraps pathrs_inroot_hardlink.
208-
func InRootHardlink(rootFd uintptr, path, target string) error {
209-
cPath := C.CString(path)
210-
defer C.free(unsafe.Pointer(cPath))
208+
func InRootHardlink(rootFd uintptr, target, linkpath string) error {
209+
cLinkPath := C.CString(linkpath)
210+
defer C.free(unsafe.Pointer(cLinkPath))
211211

212212
cTarget := C.CString(target)
213213
defer C.free(unsafe.Pointer(cTarget))
214214

215-
err := C.pathrs_inroot_hardlink(C.int(rootFd), cPath, cTarget)
215+
err := C.pathrs_inroot_hardlink(C.int(rootFd), cLinkPath, cTarget)
216216
return fetchError(err)
217217
}
218218

go-pathrs/root_linux.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,26 +277,26 @@ func (r *Root) Mknod(path string, mode os.FileMode, dev uint64) error {
277277
}
278278

279279
// Symlink creates a symlink within a [Root]'s directory tree. The symlink is
280-
// created at path and is a link to target.
280+
// created at newname and is a link to oldname.
281281
//
282282
// This is effectively equivalent to [os.Symlink].
283-
func (r *Root) Symlink(path, target string) error {
283+
func (r *Root) Symlink(oldname, newname string) error {
284284
_, err := fdutils.WithFileFd(r.inner, func(rootFd uintptr) (struct{}, error) {
285-
err := libpathrs.InRootSymlink(rootFd, path, target)
285+
err := libpathrs.InRootSymlink(rootFd, oldname, newname)
286286
return struct{}{}, err
287287
})
288288
return err
289289
}
290290

291291
// Hardlink creates a hardlink within a [Root]'s directory tree. The hardlink
292-
// is created at path and is a link to target. Both paths are within the
292+
// is created at newname and is a link to oldname. Both paths are within the
293293
// [Root]'s directory tree (you cannot hardlink to a different [Root] or the
294294
// host).
295295
//
296296
// This is effectively equivalent to [os.Link].
297-
func (r *Root) Hardlink(path, target string) error {
297+
func (r *Root) Hardlink(oldname, newname string) error {
298298
_, err := fdutils.WithFileFd(r.inner, func(rootFd uintptr) (struct{}, error) {
299-
err := libpathrs.InRootHardlink(rootFd, path, target)
299+
err := libpathrs.InRootHardlink(rootFd, oldname, newname)
300300
return struct{}{}, err
301301
})
302302
return err

include/pathrs.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,9 @@ int pathrs_inroot_mknod(int root_fd,
520520
* the system errno(7) value associated with the error, etc), use
521521
* pathrs_errorinfo().
522522
*/
523-
int pathrs_inroot_symlink(int root_fd, const char *path, const char *target);
523+
int pathrs_inroot_symlink(int root_fd,
524+
const char *target,
525+
const char *linkpath);
524526

525527
/**
526528
* Create a hardlink within the rootfs referenced by root_fd. Both the hardlink
@@ -535,7 +537,9 @@ int pathrs_inroot_symlink(int root_fd, const char *path, const char *target);
535537
* the system errno(7) value associated with the error, etc), use
536538
* pathrs_errorinfo().
537539
*/
538-
int pathrs_inroot_hardlink(int root_fd, const char *path, const char *target);
540+
int pathrs_inroot_hardlink(int root_fd,
541+
const char *target,
542+
const char *linkpath);
539543

540544
/**
541545
* Create a new (custom) procfs root handle.

src/capi/core.rs

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -623,24 +623,42 @@ utils::symver! {
623623
#[no_mangle]
624624
pub unsafe extern "C" fn pathrs_inroot_symlink(
625625
root_fd: CBorrowedFd<'_>,
626-
path: *const c_char,
627626
target: *const c_char,
627+
linkpath: *const c_char,
628628
) -> c_int {
629629
|| -> Result<_, Error> {
630630
let root_fd = root_fd.try_as_borrowed_fd()?;
631631
let root = RootRef::from_fd(root_fd);
632-
let path = unsafe { utils::parse_path(path)? }; // SAFETY: C caller guarantees path is safe.
633632
let target = unsafe { utils::parse_path(target)? }; // SAFETY: C caller guarantees path is safe.
634-
root.create(path, &InodeType::Symlink(target.into()))
633+
let linkpath = unsafe { utils::parse_path(linkpath)? }; // SAFETY: C caller guarantees path is safe.
634+
root.create(linkpath, &InodeType::Symlink(target.into()))
635635
}()
636636
.into_c_return()
637637
}
638638
utils::symver! {
639-
fn pathrs_inroot_symlink <- (pathrs_inroot_symlink, version = "LIBPATHRS_0.2", default);
639+
fn pathrs_inroot_symlink <- (pathrs_inroot_symlink, version = "LIBPATHRS_0.3", default);
640+
}
641+
642+
/// A compatibility shim for `pathrs_inroot_symlink`, which previously took its
643+
/// arguments in the wrong order.
644+
///
645+
/// cbindgen:ignore
646+
#[no_mangle]
647+
pub unsafe extern "C" fn __pathrs_inroot_symlink_v1(
648+
root_fd: CBorrowedFd<'_>,
649+
path: *const c_char,
650+
target: *const c_char,
651+
) -> c_int {
652+
pathrs_inroot_symlink(root_fd, target, path)
653+
}
654+
utils::symver! {
655+
// The old version of pathrs_inroot_symlink had "target" and "linkpath"
656+
// flipped.
657+
fn __pathrs_inroot_symlink_v1 <- (pathrs_inroot_symlink, version = "LIBPATHRS_0.2");
640658
// This symbol was renamed in libpathrs 0.2. For backward compatibility with
641659
// pre-symbol-versioned builds of libpathrs, it needs to be a default so
642660
// that loaders will pick it when searching for the unversioned name.
643-
fn pathrs_inroot_symlink <- (pathrs_symlink, version = "LIBPATHRS_0.1", default);
661+
fn __pathrs_inroot_symlink_v1 <- (pathrs_symlink, version = "LIBPATHRS_0.1", default);
644662
}
645663

646664
/// Create a hardlink within the rootfs referenced by root_fd. Both the hardlink
@@ -657,22 +675,40 @@ utils::symver! {
657675
#[no_mangle]
658676
pub unsafe extern "C" fn pathrs_inroot_hardlink(
659677
root_fd: CBorrowedFd<'_>,
660-
path: *const c_char,
661678
target: *const c_char,
679+
linkpath: *const c_char,
662680
) -> c_int {
663681
|| -> Result<_, Error> {
664682
let root_fd = root_fd.try_as_borrowed_fd()?;
665683
let root = RootRef::from_fd(root_fd);
666-
let path = unsafe { utils::parse_path(path)? }; // SAFETY: C caller guarantees path is safe.
667684
let target = unsafe { utils::parse_path(target)? }; // SAFETY: C caller guarantees path is safe.
668-
root.create(path, &InodeType::Hardlink(target.into()))
685+
let linkpath = unsafe { utils::parse_path(linkpath)? }; // SAFETY: C caller guarantees path is safe.
686+
root.create(linkpath, &InodeType::Hardlink(target.into()))
669687
}()
670688
.into_c_return()
671689
}
672690
utils::symver! {
673-
fn pathrs_inroot_hardlink <- (pathrs_inroot_hardlink, version = "LIBPATHRS_0.2", default);
691+
fn pathrs_inroot_hardlink <- (pathrs_inroot_hardlink, version = "LIBPATHRS_0.3", default);
692+
}
693+
694+
/// A compatibility shim for `pathrs_inroot_hardlink`, which previously took its
695+
/// arguments in the wrong order.
696+
///
697+
/// cbindgen:ignore
698+
#[no_mangle]
699+
pub unsafe extern "C" fn __pathrs_inroot_hardlink_v1(
700+
root_fd: CBorrowedFd<'_>,
701+
path: *const c_char,
702+
target: *const c_char,
703+
) -> c_int {
704+
pathrs_inroot_hardlink(root_fd, target, path)
705+
}
706+
utils::symver! {
707+
// The old version of pathrs_inroot_hardlink had "target" and "linkpath"
708+
// flipped.
709+
fn __pathrs_inroot_hardlink_v1 <- (pathrs_inroot_hardlink, version = "LIBPATHRS_0.2");
674710
// This symbol was renamed in libpathrs 0.2. For backward compatibility with
675711
// pre-symbol-versioned builds of libpathrs, it needs to be a default so
676712
// that loaders will pick it when searching for the unversioned name.
677-
fn pathrs_inroot_hardlink <- (pathrs_hardlink, version = "LIBPATHRS_0.1", default);
713+
fn __pathrs_inroot_hardlink_v1 <- (pathrs_hardlink, version = "LIBPATHRS_0.1", default);
678714
}

src/tests/capi/root.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ impl CapiRoot {
139139
unsafe {
140140
capi::core::pathrs_inroot_symlink(
141141
root_fd.into(),
142-
path.as_ptr(),
143142
target.as_ptr(),
143+
path.as_ptr(),
144144
)
145145
}
146146
}
@@ -149,8 +149,8 @@ impl CapiRoot {
149149
unsafe {
150150
capi::core::pathrs_inroot_hardlink(
151151
root_fd.into(),
152-
path.as_ptr(),
153152
target.as_ptr(),
153+
path.as_ptr(),
154154
)
155155
}
156156
}

0 commit comments

Comments
 (0)