Date: Mon, 8 Jan 2018 18:05:25 +0000 From: Andrew Duane <aduane@juniper.net> To: Eric van Gyzen <eric@vangyzen.net>, "Rodney W. Grimes" <freebsd-rwg@pdx.rh.CN85.dnsmgr.net>, Eugene Grosbein <eugen@grosbein.net> Cc: Yuri <yuri@rawbw.com>, Brooks Davis <brooks@freebsd.org>, Ian Lepore <ian@freebsd.org>, Alan Somers <asomers@freebsd.org>, Freebsd hackers list <freebsd-hackers@freebsd.org> Subject: RE: Is it considered to be ok to not check the return code of close(2) in base? Message-ID: <SN1PR0501MB212559C6D852EBEEA3A89C7CCE130@SN1PR0501MB2125.namprd05.prod.outlook.com> In-Reply-To: <df6f98a5-76db-d6d8-6321-d35b59eeec22@vangyzen.net> References: <201801081655.w08GtO3D022568@pdx.rh.CN85.dnsmgr.net> <df6f98a5-76db-d6d8-6321-d35b59eeec22@vangyzen.net>
next in thread | previous in thread | raw e-mail | index | archive | help
> -----Original Message-----
> From: owner-freebsd-hackers@freebsd.org [mailto:owner-freebsd-
> hackers@freebsd.org] On Behalf Of Eric van Gyzen
> Sent: Monday, January 8, 2018 12:52 PM
> To: Rodney W. Grimes <freebsd-rwg@pdx.rh.CN85.dnsmgr.net>; Eugene
> Grosbein <eugen@grosbein.net>
> Cc: Yuri <yuri@rawbw.com>; Brooks Davis <brooks@freebsd.org>; Ian
> Lepore <ian@freebsd.org>; Alan Somers <asomers@freebsd.org>; Freebsd
> hackers list <freebsd-hackers@freebsd.org>
> Subject: Re: Is it considered to be ok to not check the return code of cl=
ose(2)
> in base?
>=20
> On 01/08/2018 10:55, Rodney W. Grimes wrote:
> >> 08.01.2018 23:13, Eric van Gyzen wrote:
> >>
> >>> Right, which is the reason such bugs are hard to diagnose.
> >>> Optionally killing the process on close->EBADF would help find buggy
> >>> code when another thread did NOT re-open the file descriptor between
> >>> the two close calls.
> >>
> >> Wouldn't "close(f); assert(errno !=3D EBADF);" be better?
>=20
> Putting the code in one place is far better than putting it in N places..=
.after
> /finding/ those N places. Indeed, the purpose of this code is to help pe=
ople
> find those places, even in their own code, outside of base.
>=20
> > Or even
> > #ifdef DEBUG_CLOSE
> > #define close(f) close(f); assert(errno !=3D EBADF);
> > #endif
>=20
> errno could have been EBADF before the close(). A successful close() doe=
s
> not modify errno. So, this would have be larger, making it even more
> unpalatable.
>=20
> > Then the people that want to go chasing these errors can, and the rest
> > of us are untouched.
>=20
> Every mention in this thread of killing the process has called it optiona=
l.
> Tools, not policy.
>=20
> Eric
Of course, my OCD will kick in and say this would need to be something like=
:
#ifdef DEBUG_CLOSE
#define close(f) do {if (close(f) < 0) assert(errno !=3D EBADF); } while (0=
)
#endif
Have to watch those macro replacements like "if (need_to_close) close(f);".=
And the close succeeding :-)
....................................
Andrew L. Duane - Principal Resident Engineer
AT&T Advanced Services Technical Lead
Juniper Quality Ambassador
m +1 603.770.7088
o +1 408.933.6944 (2-6944)
skype: andrewlduane
aduane@juniper.net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?SN1PR0501MB212559C6D852EBEEA3A89C7CCE130>