From kde-frameworks-devel Fri Jan 01 19:14:25 2016 From: "David Faure" Date: Fri, 01 Jan 2016 19:14:25 +0000 To: kde-frameworks-devel Subject: Re: Review Request 126474: Port QRegExp to QRegularExpression in kshorturifilter Message-Id: <20160101191425.30660.68704 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-frameworks-devel&m=145171195306513 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============6554645464795620948==" --===============6554645464795620948== Content-Type: multipart/alternative; boundary="===============3591189350391929890==" --===============3591189350391929890== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Jan. 1, 2016, 5:15 p.m., David Faure wrote: > > src/urifilters/shorturi/kshorturifilter.cpp, line 58 > > > > > > "despite" sounds like the api docs say that it's not thread safe. AFAICS the docs don't say anything either way. I agree that one shouldn't assume thread-safety unless explicitly documented, but I think it's just an omission in the doc, the whole point of the QRegularExpression API is thread safety. I talked to Giuseppe, he'll update the docu to mention thread safety. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126474/#review90403 ----------------------------------------------------------- On Dec. 28, 2015, 2:20 p.m., David Edmundson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126474/ > ----------------------------------------------------------- > > (Updated Dec. 28, 2015, 2:20 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > ------- > > A static QRegExp was used but it is not thread safe. QRegularExpression > seems to be. > > BUG: 352356 > > > Diffs > ----- > > src/urifilters/shorturi/kshorturifilter.cpp 6002ec6925c0acdd20a053f98baca46863f69fa6 > > Diff: https://git.reviewboard.kde.org/r/126474/diff/ > > > Testing > ------- > > I ran the autotests which includes urifilter and I've run krunner which uses it extensively. > > > Thanks, > > David Edmundson > > --===============3591189350391929890== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126474/

On January 1st, 2016, 5:15 p.m. UTC, David Faure wrote:

src/urifilters/shorturi/kshorturifilter.cpp (Diff revision 2)

static QRegExp sEnvVarExp (QL1S("\\$[a-zA-Z_][a-zA-Z0-9_]*"));

57

static QRegExp sEnvVarExp (QL1S("\\$[a-zA-Z_][a-zA-Z0-9_]*"));

58

//despite current QRegularExpression documentation, QRegularExpression::match appears to be thread safe and safe for our static usage

"despite" sounds like the api docs say that it's not thread safe. AFAICS the docs don't say anything either way. I agree that one shouldn't assume thread-safety unless explicitly documented, but I think it's just an omission in the doc, the whole point of the QRegularExpression API is thread safety.

I talked to Giuseppe, he'll update the docu to mention thread safety.

- David

On December 28th, 2015, 2:20 p.m. UTC, David Edmundson wrote:

Review request for KDE Frameworks.

By David Edmundson.

Updated Dec. 28, 2015, 2:20 p.m.

Repository: kio

Description

A static QRegExp was used but it is not thread safe. QRegularExpression
seems to be.
BUG: 352356

Testing

I ran the autotests which includes urifilter and I've run krunner which uses it extensively.

Diffs

src/urifilters/shorturi/kshorturifilter.cpp (6002ec6925c0acdd20a053f98baca46863f69fa6)

--===============3591189350391929890==-- --===============6554645464795620948== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KS2RlLWZyYW1l d29ya3MtZGV2ZWwgbWFpbGluZyBsaXN0CktkZS1mcmFtZXdvcmtzLWRldmVsQGtkZS5vcmcKaHR0 cHM6Ly9tYWlsLmtkZS5vcmcvbWFpbG1hbi9saXN0aW5mby9rZGUtZnJhbWV3b3Jrcy1kZXZlbAo= --===============6554645464795620948==--