This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 4/5] Fix deadlock in _int_free consistency check
- From: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- To: DJ Delorie <dj at redhat dot com>
- Cc: "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, nd <nd at arm dot com>
- Date: Thu, 12 Oct 2017 21:35:06 +0000
- Subject: Re: [PATCH 4/5] Fix deadlock in _int_free consistency check
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Wilco dot Dijkstra at arm dot com;
- Nodisclaimer: True
- References: <DB6PR0801MB20537B594B5629491605CD74834B0@DB6PR0801MB2053.eurprd08.prod.outlook.com> (message from Wilco Dijkstra on Thu, 12 Oct 2017 09:35:41 +0000),<xnbmlc6qhj.fsf@greed.delorie.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
DJ Delorie wrote:
> I think the bug can be fixed by only changing the sense of the have_lock
> condition:
> - if (!have_lock
> + if (have_lock
Yes it could but then it's still impossible to follow the logic... The only reason
I spotted the bug was because I refactored the code. Then the bug suddenly
became very obvious. I think locking as a side effect in conditions is something
we should avoid.
So my variant or this one are reasonable ways to do it:
if (error detected)
{
if (have_lock)
/* we had the lock during the test above, so the test is valid,
and the error we detect is valid. */
print error message
else
/* we didn't have the lock, so aquire it and repeat the test. If
the error is still present, fail. */
get lock, repeat test, maybe print error message
}
I suppose we could also print the error once at the end:
if (error detected)
{
bool fail = true;
if (!have_lock)
{
get_lock
fail = repeat test
unlock
}
if (fail)
print error
}
Wilco