[RFC] Make the report_error(), get_error_string() code thread safe

Berthold Stoeger bstoeger at mail.tuwien.ac.at
Wed Nov 1 09:09:56 PDT 2017


On Mittwoch, 1. November 2017 15:28:28 CET Dirk Hohndel wrote:
> On Wed, Nov 01, 2017 at 01:54:17PM +0100, Berthold Stoeger wrote:
> > 1) errorhelper.c is turned into C++ code.
> 
> Linus will love that :-)

I sense some sarcasm. ;)
I can relate. When using a full blown cross-platform toolkit like Qt you can 
expect some contagiousness. I have a feeling that Qt is particularly 
contagious, for better or worse.

> > This necessitates a kludge of declaring get_error_string() only for C++
> > code and makes dive.h inconsistent.
> 
> Well, if it's only used in C++ code then it maybe shouldn't be declared in
> dive.h in the first place.

"core/helpers.h" seems like the place for C++ helpers.

> > -	struct membuffer *buf = &error_string_buffer;
> > +	{
> 
> Why is there another indentation / opening brace?
> 
> > +		QMutexLocker lock(&error_string_buffer_mutex);
> > +		struct membuffer *buf = &error_string_buffer;
> > 
> > -	/* Previous unprinted errors? Add a newline in between */
> > -	if (buf->len)
> > -		put_bytes(buf, "\n", 1);
> > -	VA_BUF(buf, fmt);
> > -	mb_cstring(buf);
> > +		/* Previous unprinted errors? Add a newline in between */
> > +		if (buf->len)
> > +			put_bytes(buf, "\n", 1);
> > +		VA_BUF(buf, fmt);
> > +		mb_cstring(buf);
> > +	}
> 
> My guess is you are doing this to destroy the QMutexLocker. I think the
> code would be much easier to read if you simply called lock->unlock();

Indeed... AFAIK this is idiomatic (or idiosyncratic?) C++. The idea is that 
any function call can throw an exception. The brace-close makes the lock-guard 
go out of scope and thus releases the lock.

Since this is no-exception code, I replaced it by a .lock()/.unlock() pair, 
which is hopefully easier on the eyes.

Updated patch attached.

Berthold
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-the-report_error-get_error_string-code-thread-s.patch
Type: text/x-patch
Size: 6559 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20171101/55599f90/attachment.bin>


More information about the subsurface mailing list