On Mon, Aug 04, 2025 at 04:52:29PM +0100, Al Viro wrote:
On Mon, Aug 04, 2025 at 02:33:13PM +0200, Christian Brauner wrote:
guard(spinlock)(&files->file_lock); err = expand_files(files, fd); if (unlikely(err < 0))
goto out_unlock;
return do_dup2(files, file, fd, flags);
return err;
err = do_dup2(files, file, fd, flags);
if (err < 0)
return err;
-out_unlock:
spin_unlock(&files->file_lock);
return err;
return 0;
}
NAK. This is broken - do_dup2() drops ->file_lock. And that's why I
Right, I missed that. Just say it's broken. You don't need to throw around NAKs pointlessly. It's pretty clear when to drop ptaches without throwing the meat cleaver through the room.
loathe the guard() - it's too easy to get confused *and* assume that
The calling conventions of do_dup2() are terrible. The only reason it drops file_lock itself instead of leaving it to the two callers that have to acquire it anyway is because it wants to call filp_close() if there's already a file on that fd.
And really the side-effect of dropping a lock implicitly is nasty especially when the function doesn't even indicate that it does that in it's name.
And guards are great.