Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
bug-hurd
Top
][
All Lists
Date Prev
][
Date Next
][
Thread Prev
][
Thread Next
][
Date Index
][
Thread Index
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
From
Samuel Thibault
Subject
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Date
Sat, 14 Feb 2026 14:54:13 +0100
Applied, thanks!
Milos Nikic, le jeu. 12 févr. 2026 16:29:45 -0800, a ecrit:
Hey Samuel,
Yeah we don't really need to do it, i was doing it for readability.
(in my mind these functions are getting too large and maybe it would be worth
splitting them into smaller functions...but that is a separate thing).
But yeah assert works as well, and it has no impact on performance.
here is the updated patch, where assert_backtrace is used instead of an
explicit if statement.
Let me know.
Thanks a lot,
Milos
On Thu, Feb 12, 2026 at 3:39 PM Samuel Thibault <[1]samuel.thibault@gnu.org>
wrote:
Hello,
Milos Nikic, le jeu. 12 févr. 2026 06:30:45 -0800, a ecrit:
> We don't always undo the link increases in error cases.
> Added logic the undo the changes to link count if an
> error occurred in those cases.
> ---
> libdiskfs/dir-rename.c | 12 ++++++++++--
> libdiskfs/dir-renamed.c | 29 +++++++++++++++++++++++++++--
> 2 files changed, 37 insertions(+), 4 deletions(-)
> diff --git a/libdiskfs/dir-rename.c b/libdiskfs/dir-rename.c
> index c8bb0dad..7953132b 100644
> --- a/libdiskfs/dir-rename.c
> +++ b/libdiskfs/dir-rename.c
> @@ -188,12 +188,20 @@ diskfs_S_dir_rename (struct protid *fromcred,
> diskfs_node_update (tdp, 1);
> pthread_mutex_unlock (&tdp->lock);
> - pthread_mutex_unlock (&fnp->lock);
> if (err)
> {
> - diskfs_nrele (fnp);
> + if (fnp->dn_stat.st_nlink > 0)
Do we really need to check these? We have increased it just above, and
haven't released the mutex. We're actually the owner of that reference,
it'd rather be an assert.
> + {
> + fnp->dn_stat.st_nlink--;
> + fnp->dn_set_ctime = 1;
> + if (diskfs_synchronous)
> + diskfs_node_update (fnp, 1);
> + }
> + diskfs_drop_dirstat (tdp, ds);
> + diskfs_nput (fnp);
> return err;
> }
> + pthread_mutex_unlock (&fnp->lock);
> /* We now hold no locks */
> diff --git a/libdiskfs/dir-renamed.c b/libdiskfs/dir-renamed.c
> index 1f3f3de4..532ff998 100644
> --- a/libdiskfs/dir-renamed.c
> +++ b/libdiskfs/dir-renamed.c
> @@ -159,6 +159,13 @@ diskfs_rename_dir (struct node *fdp, struct node
*fnp, const char *fromname,
> assert_backtrace (err != ENOENT);
> if (err)
> {
> + if (tdp->dn_stat.st_nlink > 0)
> + {
> + tdp->dn_stat.st_nlink--;
> + tdp->dn_set_ctime = 1;
> + if (diskfs_synchronous)
> + diskfs_node_update (tdp, 1);
> + }
> diskfs_drop_dirstat (fnp, tmpds);
> goto out;
> }
> @@ -168,7 +175,16 @@ diskfs_rename_dir (struct node *fdp, struct node
*fnp, const char *fromname,
> if (diskfs_synchronous)
> diskfs_file_update (fnp, 1);
> if (err)
> - goto out;
> + {
> + if (tdp->dn_stat.st_nlink > 0)
> + {
> + tdp->dn_stat.st_nlink--;
> + tdp->dn_set_ctime = 1;
> + if (diskfs_synchronous)
> + diskfs_node_update (tdp, 1);
> + }
> + goto out;
> + }
> fdp->dn_stat.st_nlink--;
> fdp->dn_set_ctime = 1;
> @@ -213,7 +229,16 @@ diskfs_rename_dir (struct node *fdp, struct node
*fnp, const char *fromname,
> }
> if (err)
> - goto out;
> + {
> + if (fnp->dn_stat.st_nlink > 0)
> + {
> + fnp->dn_stat.st_nlink--;
> + fnp->dn_set_ctime = 1;
> + if (diskfs_synchronous)
> + diskfs_node_update (fnp, 1);
> + }
> + goto out;
> + }
> /* 4: Remove the entry in fdp. */
> ds = buf;
> --
> 2.52.0
--
Samuel
"...Unix, MS-DOS, and Windows NT (also known as the Good, the Bad, and
the Ugly)."
(By Matt Welsh)
References:
[1]
mailto:samuel.thibault@gnu.org
From 253032a144192aa34fb456c072c7e30ec96a9662 Mon Sep 17 00:00:00 2001
From: Milos Nikic
Date: Thu, 12 Feb 2026 06:06:16 -0800
Subject: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
We don't always undo the link increases in error cases.
Added logic the undo the changes to link count if an
error occurred in those cases.
---
libdiskfs/dir-rename.c | 10 ++++++++--
libdiskfs/dir-renamed.c | 23 +++++++++++++++++++++--
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/libdiskfs/dir-rename.c b/libdiskfs/dir-rename.c
index c8bb0dad..6b4e9a7d 100644
--- a/libdiskfs/dir-rename.c
+++ b/libdiskfs/dir-rename.c
@@ -188,12 +188,18 @@ diskfs_S_dir_rename (struct protid *fromcred,
diskfs_node_update (tdp, 1);
pthread_mutex_unlock (&tdp->lock);
- pthread_mutex_unlock (&fnp->lock);
if (err)
- diskfs_nrele (fnp);
+ assert_backtrace (fnp->dn_stat.st_nlink > 0);
+ fnp->dn_stat.st_nlink--;
+ fnp->dn_set_ctime = 1;
+ if (diskfs_synchronous)
+ diskfs_node_update (fnp, 1);
+ diskfs_drop_dirstat (tdp, ds);
+ diskfs_nput (fnp);
return err;
+ pthread_mutex_unlock (&fnp->lock);
/* We now hold no locks */
diff --git a/libdiskfs/dir-renamed.c b/libdiskfs/dir-renamed.c
index 1f3f3de4..a68dc07c 100644
--- a/libdiskfs/dir-renamed.c
+++ b/libdiskfs/dir-renamed.c
@@ -159,6 +159,11 @@ diskfs_rename_dir (struct node *fdp, struct node *fnp,
const char *fromname,
assert_backtrace (err != ENOENT);
if (err)
+ assert_backtrace (tdp->dn_stat.st_nlink > 0);
+ tdp->dn_stat.st_nlink--;
+ tdp->dn_set_ctime = 1;
+ if (diskfs_synchronous)
+ diskfs_node_update (tdp, 1);
diskfs_drop_dirstat (fnp, tmpds);
goto out;
@@ -168,7 +173,14 @@ diskfs_rename_dir (struct node *fdp, struct node *fnp,
const char *fromname,
if (diskfs_synchronous)
diskfs_file_update (fnp, 1);
if (err)
- goto out;
+ {
+ assert_backtrace (tdp->dn_stat.st_nlink > 0);
+ tdp->dn_stat.st_nlink--;
+ tdp->dn_set_ctime = 1;
+ if (diskfs_synchronous)
+ diskfs_node_update (tdp, 1);
+ goto out;
+ }
fdp->dn_stat.st_nlink--;
fdp->dn_set_ctime = 1;
@@ -213,7 +225,14 @@ diskfs_rename_dir (struct node *fdp, struct node *fnp,
const char *fromname,
if (err)
- goto out;
+ {
+ assert_backtrace (fnp->dn_stat.st_nlink > 0);
+ fnp->dn_stat.st_nlink--;
+ fnp->dn_set_ctime = 1;
+ if (diskfs_synchronous)
+ diskfs_node_update (fnp, 1);
+ goto out;
+ }
/* 4: Remove the entry in fdp. */
ds = buf;
--
2.52.0
--
Samuel
(03:13:14)
(03:13:19)
(03:13:25)
(03:13:38)
Prev in Thread
Current Thread
Next in Thread
[PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Milos Nikic
2026/02/12
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Samuel Thibault
2026/02/12
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Milos Nikic
2026/02/12
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Samuel Thibault
<=
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Samuel Thibault
2026/02/14
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Milos Nikic
2026/02/15
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Samuel Thibault
2026/02/15
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Milos Nikic
2026/02/27
Prev by Date:
Re: [PATCH v2 glibc 1/1] hurd: calling alarm() whilst handling SIGALRM can deadlock.
Next by Date:
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Previous by thread:
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Next by thread:
Re: [PATCH] libdiskfs: fix cleanups in dir-rename and dir-renamed
Index(es):
Date
Thread