[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