[PATCH v1] proto: fix file descriptor leak

[PATCH v1] proto: fix file descriptor leak

[Luca Di Maio]

fix leak of pathfd introduced in commit 8a4ea72724930cfe262ccda03028264e1a81b145

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
---
 mkfs/proto.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mkfs/proto.c b/mkfs/proto.c
index 2b29240d..1a7b3586 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -1772,6 +1772,7 @@ handle_direntry(
 	create_nondir_inode(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
 			    rdev, fd, fname);
 out:
+	close(pathfd);
 	/* Reset path_buf to original */
 	path_buf[path_len] = '\0';
 }
--
2.51.0

Re: [PATCH v1] proto: fix file descriptor leak

[Luca Di Maio]

Hi all,
I apologize for introducing a file descriptor leak in commit 8a4ea727.
The pathfd was not being properly closed.

I've submitted a fix (v1 patch) that adds the missing close() call.
I'd deeply sorry for the oversight.

L.

On Fri, Oct 24, 2025 at 09:36:48PM +0200, Luca Di Maio wrote:
> fix leak of pathfd introduced in commit 8a4ea72724930cfe262ccda03028264e1a81b145
>
> Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
> ---
>  mkfs/proto.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 2b29240d..1a7b3586 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -1772,6 +1772,7 @@ handle_direntry(
>  	create_nondir_inode(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
>  			    rdev, fd, fname);
>  out:
> +	close(pathfd);
>  	/* Reset path_buf to original */
>  	path_buf[path_len] = '\0';
>  }
> --
> 2.51.0

Re: [PATCH v1] proto: fix file descriptor leak

[Darrick J. Wong]

On Fri, Oct 24, 2025 at 09:36:48PM +0200, Luca Di Maio wrote:
> fix leak of pathfd introduced in commit 8a4ea72724930cfe262ccda03028264e1a81b145
> 
> Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>

A few points:

First, everyone makes mistakes, don't worry about it. :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Second, the canonical format for kernel-ish formats is most probably
something like:

Cc: <linux-xfs@vger.kernel.org> # v6.17.0
Fixes: 8a4ea72724930c ("proto: add ability to populate a filesystem from a directory")

but this is xfsprogs, so there isn't any formally established
convention aside from "Darrick copies the kernel style and the
complaining hasn't been harsh enough for him to stop".

Third, does anyone actually have a script to generate these git
trailers?  I set user.stableSubmissionTarget = linux-xfs@vger.kernel.org
in gitconfig and use this:

#!/bin/bash

# Cite a list of git hashes in Fixes: tag format.

if [ -z "$1" ] || [ "$1" = "--help" ]; then
	echo "Usage: $0 commit [commits...]"
	exit 1
fi

stable_email="$(git config user.stableSubmissionTarget)"

# Find the most recent version tag for this commit.  Assumes that version tags
# start with v and a digit.
tag_for_commit() {
	local arg="$1"
	local tag
	local release_tag
	local rc_suffix
	local version_prefix

	# Find the first tag created after this commit.
	version_prefix="$(git config versionsort.prefix)"
	test -z "${version_prefix}" && version_prefix="v"
	tag="$(git tag -l --contains "${arg}" --sort version:refname | \
		grep "^${version_prefix}[0-9]" | \
		head -n 1)"
	test -z "${tag}" && return 1

	# Strip off any version suffix (presumably "-rcX") to construct what
	# ought to be the tag name for the final release.
	rc_suffix="$(git config versionsort.suffix)"
	# shellcheck disable=SC2001
	release_tag="$(echo "${tag}" | sed -e "s/${rc_suffix}.*//g")"

	# If the release tag actually exists, print that instead of the first
	# tag.  Otherwise, print that first tag.  We'd rather print "v5.4" than
	# the more accurate "v5.4-rc1" because stable backports target
	# releases, not -rcX.
	if [ -n "$(git tag -l "${release_tag}" 2>/dev/null)" ]; then
		echo "${release_tag}"
	else
		echo "${tag}"
	fi
	return 0
}

gitlog=(git log --format='Fixes: %h ("%s")')
for arg in "$@"; do
	if [ -n "${stable_email}" ]; then
		fixed_in="$(tag_for_commit "${arg}")" && \
			echo "Cc: <${stable_email}> # ${fixed_in}"
	fi
	"${gitlog[@]}" "${arg}^1..${arg}"
done

--D

> ---
>  mkfs/proto.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 2b29240d..1a7b3586 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -1772,6 +1772,7 @@ handle_direntry(
>  	create_nondir_inode(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
>  			    rdev, fd, fname);
>  out:
> +	close(pathfd);
>  	/* Reset path_buf to original */
>  	path_buf[path_len] = '\0';
>  }
> --
> 2.51.0

Re: [PATCH v1] proto: fix file descriptor leak

[Luca Di Maio]

On Fri, Oct 24, 2025 at 02:35:11PM -0700, Darrick J. Wong wrote:
> On Fri, Oct 24, 2025 at 09:36:48PM +0200, Luca Di Maio wrote:
> > fix leak of pathfd introduced in commit 8a4ea72724930cfe262ccda03028264e1a81b145
> >
> > Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
>
> A few points:
>
> First, everyone makes mistakes, don't worry about it. :)
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thanks Darrick :)

> Second, the canonical format for kernel-ish formats is most probably
> something like:
>
> Cc: <linux-xfs@vger.kernel.org> # v6.17.0
> Fixes: 8a4ea72724930c ("proto: add ability to populate a filesystem from a directory")
>
> but this is xfsprogs, so there isn't any formally established
> convention aside from "Darrick copies the kernel style and the
> complaining hasn't been harsh enough for him to stop".
>

If it's needed I can re-rubmit with the new commit message, let me know

Thanks
L.

>
> > ---
> >  mkfs/proto.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/mkfs/proto.c b/mkfs/proto.c
> > index 2b29240d..1a7b3586 100644
> > --- a/mkfs/proto.c
> > +++ b/mkfs/proto.c
> > @@ -1772,6 +1772,7 @@ handle_direntry(
> >  	create_nondir_inode(mp, pip, fsxp, mode, creds, xname, flags, file_stat,
> >  			    rdev, fd, fname);
> >  out:
> > +	close(pathfd);
> >  	/* Reset path_buf to original */
> >  	path_buf[path_len] = '\0';
> >  }
> > --
> > 2.51.0