D13586: Ref KConfig whilst we're using it

2018-06-19 Thread Wolfgang Bauer
wbauer added a comment.


  In D13586#279538 , @fvogt wrote:
  
  > In D13586#279514 , @ngraham 
wrote:
  >
  > > Could this have any relationship to 
https://bugs.kde.org/show_bug.cgi?id=395401?
  >
  >
  > In theory yes, this is undefined behaviour and could cause everything 
between making a pixel slightly darker or crashing the harddrive.
  >
  > In practice I doubt it - this doesn't touch any of the code related to 
saving.
  
  
  But that bug is about the settings not being applied on login.
  Comment#0 says that the settings are in fact saved correctly.

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: wbauer, ngraham, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-19 Thread Fabian Vogt
fvogt added a comment.


  > Your shared pointer still goes out of scope, the group's handle to the 
kconfig object is still the same.
  
  If I read the code correctly, the group keeps the shared pointer, which means 
it owns a reference.

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: ngraham, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-19 Thread David Edmundson
davidedmundson abandoned this revision.
davidedmundson added a comment.


  I failed to use arc. 
  See https://phabricator.kde.org/D13599

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: ngraham, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-19 Thread David Edmundson
davidedmundson added a comment.


  > I guess KSharedConfig would work as well, that way the KConfig keeps a 
reference alive.
  
  I'm not convinced that
  
  auto group = KSharedConfig::openConfig().group() 
  would be any different.
  
  Your shared pointer still goes out of scope, the group's handle to the 
kconfig object is still the same.
  
  but I can change anyway

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: ngraham, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread Fabian Vogt
fvogt added a comment.


  In D13586#279514 , @ngraham wrote:
  
  > Could this have any relationship to 
https://bugs.kde.org/show_bug.cgi?id=395401?
  
  
  In theory yes, this is undefined behaviour and could cause everything between 
making a pixel slightly darker or crashing the harddrive.
  
  In practice I doubt it - this doesn't touch any of the code related to saving.

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: ngraham, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread Nathaniel Graham
ngraham added a comment.


  Could this have any relationship to 
https://bugs.kde.org/show_bug.cgi?id=395401?

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: ngraham, fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread Fabian Vogt
fvogt added a comment.


  I guess KSharedConfig would work as well, that way the KConfig keeps a 
reference alive.

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, #plasma
Cc: fvogt, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D13586: Ref KConfig whilst we're using it

2018-06-18 Thread David Edmundson
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  In current code we would have a KConfigGroup with a dangling KConfig
  deleted after the RHS for the group fetch has finished.
  
  BUG: 394534

TEST PLAN
  Wrote minimal test case of code
  It produced a valgrind warning (weirdly didn't crash though)
  Modified to correct version
  No longer any warnings

REPOSITORY
  R119 Plasma Desktop

BRANCH
  davidedmundson/kcm_clock_qml

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

AFFECTED FILES
  kcms/input/backends/x11/x11_backend.cpp

To: davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart