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>
index | next in thread | previous in thread | raw e-mail
> -----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 close(2)
> in base?
>
> 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 != EBADF);" be better?
>
> 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 people
> find those places, even in their own code, outside of base.
>
> > Or even
> > #ifdef DEBUG_CLOSE
> > #define close(f) close(f); assert(errno != EBADF);
> > #endif
>
> errno could have been EBADF before the close(). A successful close() does
> not modify errno. So, this would have be larger, making it even more
> unpalatable.
>
> > Then the people that want to go chasing these errors can, and the rest
> > of us are untouched.
>
> Every mention in this thread of killing the process has called it optional.
> Tools, not policy.
>
> 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 != 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
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?SN1PR0501MB212559C6D852EBEEA3A89C7CCE130>