14 Feb
2025
14 Feb
'25
9:34 p.m.
On Fri Feb 14 16:04:50 2025 +0000, Rémi Bernon wrote:
The comment above still applies and I don't think we should be leaking the handle or using the fd cache her. Then we don't need to keep or leak the handle and this could still thus be simplified to:
if (ret || server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL )) fd = -1; if (InterlockedCompareExchange( &device, fd, -2 ) != -2 && fd != -1) close( fd ); /* someone beat us to it */ if (!ret) NtClose( handle );Optionally with an `assert( fd == -1 || needs_close );` if you think it's worth being careful but I don't think it's necessary.
I don't see why this is an improvement? There's not any problem with "leaking" an object that lasts the entire duration of the process (we do it elsewhere), and if there was this would still mean we're "leaking" the fd. This just makes the code more unidiomatic and adds reliance on implementation details.