Hello,
On Sat, 7 Nov 2026 15:52:52 Samuel Thibault wrote:
> Mmm, we should probably be able to factorize iohelp_free_iouser (user);
> by having at the exit place a if (user) iohelp_free_iouser (user); and
> make the success path set user = NULL?
>
> You'll then be able to use the simple-to-read
>
> if (!err)
> err = do_something();
>
> if (!err)
> err = do_something();
>
> etc.
>
> if (err)
> {
> errno = err
> goto out;
> }
I think we can factor out the error-handling code in this way.
I’ve tried to make the code in libnetfs/io_duplicate.c resemble the code in
libdiskfs/io_duplicate.c. In libdiskfs, the refcount_ref function is called
after the mutex has been locked; perhaps we should do the same in libnetfs.
Thanks,
--
Mikhail Karpov
From db323186dcda64f5cc4535b679082d88d76b9c50 Mon Sep 17 00:00:00 2001
From: Mikhail Karpov <[email protected]>
Date: Sun, 8 Nov 2026 16:20:22 +0700
Subject: [PATCH 2/2] Added checks for netfs_make_peropen and netfs_make_protid
return value in several place
---
libnetfs/dir-lookup.c | 50 +++++++++++++++++++++++----------
libnetfs/dir-mkfile.c | 61 +++++++++++++++++++++++++----------------
libnetfs/fsys-getroot.c | 26 +++++++++++++++---
libnetfs/io-duplicate.c | 22 +++++++++++----
trans/fakeroot.c | 45 ++++++++++++++++++++----------
5 files changed, 141 insertions(+), 63 deletions(-)
diff --git a/libnetfs/dir-lookup.c b/libnetfs/dir-lookup.c
index faace37..b59b0d7 100644
--- a/libnetfs/dir-lookup.c
+++ b/libnetfs/dir-lookup.c
@@ -47,8 +47,9 @@ netfs_S_dir_lookup (struct protid *dircred,
char *nextname;
char *relpath;
error_t err;
+ struct peropen *newpo = NULL;
struct protid *newpi = NULL;
- struct iouser *user;
+ struct iouser *user = NULL;
if (!dircred)
return EOPNOTSUPP;
@@ -224,18 +225,22 @@ netfs_S_dir_lookup (struct protid *dircred,
err = iohelp_create_empty_iouser (&user);
if (! err)
{
- newpi = netfs_make_protid (netfs_make_peropen (dnp, 0,
- dircred->po),
- user);
+ newpo = netfs_make_peropen (dnp, 0, dircred->po);
+ if (! newpo)
+ err = errno;
+ }
+
+ if (! err)
+ {
+ newpi = netfs_make_protid (newpo, user);
if (! newpi)
- {
- err = errno;
- iohelp_free_iouser (user);
- }
+ err = errno;
}
if (! err)
{
+ user = NULL;
+ newpo = NULL;
dirport = ports_get_send_right (newpi);
@@ -301,7 +306,8 @@ netfs_S_dir_lookup (struct protid *dircred,
complete_path = translator_path;
else
{
- err = asprintf (&complete_path, "%s/%s", dircred->po->path,
+ err = asprintf (&complete_path, "%s/%s",
+ dircred->po->path,
translator_path);
if (err == -1)
{
@@ -314,7 +320,7 @@ netfs_S_dir_lookup (struct protid *dircred,
complete_path,
&np->transbox);
if (complete_path != translator_path)
- free(complete_path);
+ free (complete_path);
}
goto out;
@@ -432,15 +438,23 @@ netfs_S_dir_lookup (struct protid *dircred,
if (err)
goto out;
- newpi = netfs_make_protid (netfs_make_peropen (np, flags, dircred->po),
- user);
+ newpo = netfs_make_peropen (np, flags, dircred->po);
+ if (! newpo)
+ {
+ err = errno;
+ goto out;
+ }
+
+ newpi = netfs_make_protid (newpo, user);
if (! newpi)
{
- iohelp_free_iouser (user);
err = errno;
goto out;
}
+ user = NULL;
+ newpo = NULL;
+
mach_port_t rendezvous = MACH_PORT_NULL;
struct flock64 lock =
{
@@ -476,8 +490,10 @@ netfs_S_dir_lookup (struct protid *dircred,
else
{
newpi->po->path = NULL;
- int err2 = asprintf (&newpi->po->path, "%s/%s", dircred->po->path, relpath);
- /* If asprintf fails set path to NULL so that the if below checks errno. */
+ int err2 = asprintf (&newpi->po->path, "%s/%s",
+ dircred->po->path, relpath);
+ /* If asprintf fails set path to NULL so that the if below
+ checks errno. */
if (err2 == -1)
newpi->po->path = NULL;
}
@@ -495,6 +511,10 @@ netfs_S_dir_lookup (struct protid *dircred,
netfs_nrele (dnp);
if (newpi)
ports_port_deref (newpi);
+ if (newpo)
+ netfs_release_peropen (newpo);
+ if (user)
+ iohelp_free_iouser (user);
free (relpath);
return err;
diff --git a/libnetfs/dir-mkfile.c b/libnetfs/dir-mkfile.c
index 9036101..3549951 100644
--- a/libnetfs/dir-mkfile.c
+++ b/libnetfs/dir-mkfile.c
@@ -28,36 +28,49 @@ netfs_S_dir_mkfile (struct protid *diruser, int flags, mode_t mode,
{
error_t err;
struct node *np;
- struct iouser *user;
- struct protid *newpi;
+ struct iouser *user = NULL;
+ struct protid *newpi = NULL;
+ struct peropen *newpo = NULL;
pthread_mutex_lock (&diruser->po->np->lock);
err = netfs_attempt_mkfile (diruser->user, diruser->po->np, mode, &np);
+ if (err)
+ return err;
- if (!err)
+ /* the dir is now unlocked and NP is locked */
+ flags &= ~OPENONLY_STATE_MODES;
+ err = iohelp_dup_iouser (&user, diruser->user);
+ if (err)
+ goto out;
+
+ newpo = netfs_make_peropen (np, flags, diruser->po);
+ if (!newpo)
{
- /* the dir is now unlocked and NP is locked */
- flags &= ~OPENONLY_STATE_MODES;
- err = iohelp_dup_iouser (&user, diruser->user);
- if (! err)
- {
- newpi = netfs_make_protid (netfs_make_peropen (np, flags,
- diruser->po),
- user);
- if (newpi)
- {
- *newfile = ports_get_right (newpi);
- *newfiletype = MACH_MSG_TYPE_MAKE_SEND;
- ports_port_deref (newpi);
- }
- else
- {
- err = errno;
- iohelp_free_iouser (user);
- }
- }
- netfs_nput (np);
+ err = errno;
+ goto out;
+ }
+
+ newpi = netfs_make_protid (newpo, user);
+ if (!newpi)
+ {
+ err = errno;
+ goto out;
+ }
+
+ *newfile = ports_get_right (newpi);
+ *newfiletype = MACH_MSG_TYPE_MAKE_SEND;
+ ports_port_deref (newpi);
+
+ out:
+ if (err)
+ {
+ if (user)
+ iohelp_free_iouser (user);
+
+ if (newpo)
+ netfs_release_peropen (newpo);
}
+ netfs_nput (np);
return err;
}
diff --git a/libnetfs/fsys-getroot.c b/libnetfs/fsys-getroot.c
index fb9fef1..8c70d93 100644
--- a/libnetfs/fsys-getroot.c
+++ b/libnetfs/fsys-getroot.c
@@ -42,6 +42,7 @@ netfs_S_fsys_getroot (struct netfs_control *pt,
error_t err;
struct protid *newpi;
mode_t type;
+ struct peropen *newpo = NULL;
struct peropen peropen_context =
{
.root_parent = dotdot,
@@ -133,9 +134,20 @@ netfs_S_fsys_getroot (struct netfs_control *pt,
flags &= ~OPENONLY_STATE_MODES;
- newpi = netfs_make_protid (netfs_make_peropen (netfs_root_node, flags,
- &peropen_context),
- cred);
+ newpo = netfs_make_peropen (netfs_root_node, flags, &peropen_context);
+ if (!newpo)
+ {
+ err = errno;
+ goto out;
+ }
+
+ newpi = netfs_make_protid (newpo, cred);
+ if (!newpi)
+ {
+ err = errno;
+ goto out;
+ }
+
mach_port_deallocate (mach_task_self (), dotdot);
*do_retry = FS_RETRY_NORMAL;
@@ -146,7 +158,13 @@ netfs_S_fsys_getroot (struct netfs_control *pt,
out:
if (err)
- iohelp_free_iouser (cred);
+ {
+ iohelp_free_iouser (cred);
+
+ if (newpo)
+ netfs_release_peropen (newpo);
+ }
+
pthread_mutex_unlock (&netfs_root_node->lock);
return err;
}
diff --git a/libnetfs/io-duplicate.c b/libnetfs/io-duplicate.c
index e6ac7a1..bca0949 100644
--- a/libnetfs/io-duplicate.c
+++ b/libnetfs/io-duplicate.c
@@ -34,12 +34,24 @@ netfs_S_io_duplicate (struct protid *user,
if (err)
return err;
- refcount_ref (&user->po->refcnt);
pthread_mutex_lock (&user->po->np->lock);
+
+ refcount_ref (&user->po->refcnt);
newpi = netfs_make_protid (user->po, clone);
- *newport = ports_get_right (newpi);
+ if (newpi)
+ {
+ *newport = ports_get_right (newpi);
+ *newporttp = MACH_MSG_TYPE_MAKE_SEND;
+ ports_port_deref (newpi);
+ }
+ else
+ {
+ err = errno;
+ iohelp_free_iouser (clone);
+ refcount_deref (&user->po->refcnt);
+ }
+
pthread_mutex_unlock (&user->po->np->lock);
- *newporttp = MACH_MSG_TYPE_MAKE_SEND;
- ports_port_deref (newpi);
- return 0;
+
+ return err;
}
diff --git a/trans/fakeroot.c b/trans/fakeroot.c
index 764d6fd..c9d8eb2 100644
--- a/trans/fakeroot.c
+++ b/trans/fakeroot.c
@@ -292,8 +292,9 @@ netfs_S_dir_lookup (struct protid *diruser,
{
struct node *dnp, *np;
error_t err;
+ struct peropen *newpo = NULL;
struct protid *newpi;
- struct iouser *user;
+ struct iouser *user = NULL;
mach_port_t file;
mach_port_t idport, fsidport;
ino_t fileno;
@@ -489,24 +490,38 @@ netfs_S_dir_lookup (struct protid *diruser,
flags &= ~(O_CREAT|O_EXCL|O_NOLINK|O_NOTRANS|O_NONBLOCK);
err = iohelp_dup_iouser (&user, diruser->user);
- if (!err)
+ if (err)
+ goto lose;
+
+ newpo = netfs_make_peropen (np, flags, diruser->po);
+ if (!newpo)
{
- newpi = netfs_make_protid (netfs_make_peropen (np, flags, diruser->po),
- user);
- if (! newpi)
- {
- iohelp_free_iouser (user);
- err = errno;
- }
- else
- {
- *retry_port = ports_get_right (newpi);
- *retry_port_type = MACH_MSG_TYPE_MAKE_SEND;
- ports_port_deref (newpi);
- }
+ err = errno;
+ goto lose;
}
+ newpi = netfs_make_protid (newpo, user);
+ if (!newpi)
+ {
+ err = errno;
+ goto lose;
+ }
+
+ *retry_port = ports_get_right (newpi);
+ *retry_port_type = MACH_MSG_TYPE_MAKE_SEND;
+ ports_port_deref (newpi);
+
+
lose:
+ if (err)
+ {
+ if (user)
+ iohelp_free_iouser (user);
+
+ if (newpo)
+ netfs_release_peropen (newpo);
+ }
+
if (np != NULL)
netfs_nput (np);
return err;
--
2.43.0
