From:   David Laight <David.Laight@ACULAB.COM>
To:     'aloktiagi' <aloktiagi@gmail.com>,
        "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "keescook@chromium.org" <keescook@chromium.org>,
        "hch@infradead.org" <hch@infradead.org>,
        Tycho Andersen <tycho@tycho.pizza>
Subject: RE: [RFC 2/3] file: allow callers to free the old file descriptor
 after dup2
Thread-Topic: [RFC 2/3] file: allow callers to free the old file descriptor
 after dup2
Thread-Index: AQHZTTP1xmE+kqQuQ0egihVRbIR0J67oDplw
Date:   Thu, 2 Mar 2023 22:21:11 +0000
Message-ID: <bef11c18fa234948bcab0316418f04aa@AcuMS.aculab.com>
References: <20230302182207.456311-1-aloktiagi@gmail.com>
 <20230302182207.456311-2-aloktiagi@gmail.com>
In-Reply-To: <20230302182207.456311-2-aloktiagi@gmail.com>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: aloktiagi
> Sent: 02 March 2023 18:22
> 
> Allow callers of do_dup2 to free the old file descriptor in case they need to
> make additional operations on it.

That doesn't read right at all.

Whether or not this is a good idea (or can be done differently)
the interface is horrid.

>  	if (tofree)
> -		filp_close(tofree, files);
> +		*fdfile = tofree;

Why not:

	if (fdfile) [
		*fdfile = tofree;
	} else {
		if (tofree)
			filp_close(tofree, files);
	}

Then existing code just passes NULL and the caller can't 'forget'
to intitalise fdfile.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)