D19799: QNetworkReply was not deleted

2019-03-24 Thread Laurent Montel 
This revision was automatically updated to reflect the committed changes.
Closed by commit R134:4adbb68bf405: QNetworkReply was not deleted (authored by 
mlaurent).

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19799?vs=54387=54647

REVISION DETAIL
  https://phabricator.kde.org/D19799

AFFECTED FILES
  libdiscover/appstream/OdrsReviewsBackend.cpp
  libdiscover/backends/FlatpakBackend/FlatpakBackend.cpp

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-23 Thread Laurent Montel 
mlaurent added a comment.


  is it ok now ?

REPOSITORY
  R134 Discover Software Store

REVISION DETAIL
  https://phabricator.kde.org/D19799

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-20 Thread Laurent Montel 
mlaurent updated this revision to Diff 54387.
mlaurent added a comment.


  Fix QScopedPointer

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19799?vs=54015=54387

BRANCH
  delete_qnetwork_reply (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D19799

AFFECTED FILES
  libdiscover/appstream/OdrsReviewsBackend.cpp
  libdiscover/backends/FlatpakBackend/FlatpakBackend.cpp

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> apol wrote in FlatpakBackend.cpp:143
> This will crash here because it's passed into the connect, so it will be 
> deleted when the function leaves and accessed from the lambda.

Just move the `QScopedPointer` into the lambda, which is what I was thinking 
about (wasn't clear in my comment, sorry)

REPOSITORY
  R134 Discover Software Store

REVISION DETAIL
  https://phabricator.kde.org/D19799

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Aleix Pol Gonzalez 
apol added a comment.


  How about connecting: `connect(reply, ::finished, reply, 
::deleteLater);` after instantiating and be done with it?

REPOSITORY
  R134 Discover Software Store

REVISION DETAIL
  https://phabricator.kde.org/D19799

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Aleix Pol Gonzalez 
apol added inline comments.

INLINE COMMENTS

> FlatpakBackend.cpp:143
>  auto replyGet = get(QNetworkRequest(m_url));
> -
> +QScopedPointer 
> replyPtr(replyGet);
>  connect(replyGet, ::finished, this, [this, replyGet] {

This will crash here because it's passed into the connect, so it will be 
deleted when the function leaves and accessed from the lambda.

> FlatpakBackend.cpp:156
>  connect(replyPut, ::finished, this, [this, 
> originalUrl, fileUrl, replyPut]() {
>  if (replyPut->error() != QNetworkReply::NoError) {
>  qWarning() << "couldn't save" << originalUrl << 
> replyPut->errorString();

This will crash too.

REPOSITORY
  R134 Discover Software Store

REVISION DETAIL
  https://phabricator.kde.org/D19799

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Laurent Montel 
mlaurent updated this revision to Diff 54015.
mlaurent added a comment.


  Use QScopedPointer

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19799?vs=53998=54015

BRANCH
  delete_qnetwork_reply (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D19799

AFFECTED FILES
  libdiscover/appstream/OdrsReviewsBackend.cpp
  libdiscover/backends/FlatpakBackend/FlatpakBackend.cpp

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> OdrsReviewsBackend.cpp:186
>  m_isFetching = false;
> + reply->deleteLater();
>  return;

Can't you call this at the beginng of the method? Or let it be owned by a 
`QScopedPointer replyPtr(sender());` 
instead of calling `deleteLater()` explicitly before each return

REPOSITORY
  R134 Discover Software Store

REVISION DETAIL
  https://phabricator.kde.org/D19799

To: mlaurent, apol, #discover_software_store
Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Laurent Montel 
mlaurent updated this revision to Diff 53998.
mlaurent added a comment.


  Fix indent

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19799?vs=53990=53998

BRANCH
  delete_qnetwork_reply (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D19799

AFFECTED FILES
  libdiscover/appstream/OdrsReviewsBackend.cpp
  libdiscover/backends/FlatpakBackend/FlatpakBackend.cpp

To: mlaurent, apol, #discover_software_store
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Aleix Pol Gonzalez 
apol added inline comments.

INLINE COMMENTS

> OdrsReviewsBackend.cpp:186
>  m_isFetching = false;
> + reply->deleteLater();
>  return;

indentation

> FlatpakBackend.cpp:149
>  Q_EMIT jobFinished(false, nullptr);
> + replyGet->deleteLater();
>  return;

Indentation

REPOSITORY
  R134 Discover Software Store

REVISION DETAIL
  https://phabricator.kde.org/D19799

To: mlaurent, apol
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19799: QNetworkReply was not deleted

2019-03-16 Thread Laurent Montel 
mlaurent created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
mlaurent requested review of this revision.

REVISION SUMMARY
  delete networkreply

REPOSITORY
  R134 Discover Software Store

BRANCH
  delete_qnetwork_reply (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D19799

AFFECTED FILES
  libdiscover/appstream/OdrsReviewsBackend.cpp
  libdiscover/backends/FlatpakBackend/FlatpakBackend.cpp

To: mlaurent
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart