fvogt requested changes to this revision.
fvogt added a comment.
This revision now requires changes to proceed.


  In D11447#229385 <https://phabricator.kde.org/D11447#229385>, @graesslin 
wrote:
  
  > In D11447#229337 <https://phabricator.kde.org/D11447#229337>, @fvogt wrote:
  >
  > > In D11447#229334 <https://phabricator.kde.org/D11447#229334>, @graesslin 
wrote:
  > >
  > > > In D11447#229128 <https://phabricator.kde.org/D11447#229128>, @fvogt 
wrote:
  > > >
  > > > > If `KWorkSpace::detectPlatform(argc, argv);` were changed to edit 
`argc` and `argv` to pass `-platform foo` instead of setting `QT_QPA_PLATFORM`, 
the `qunsetenv` calls would become unnecessary.
  > > >
  > > >
  > > > but that sounds rather dangerous. I don't want to mess with the command 
line arguments
  > >
  > >
  > > Why is that more dangerous than environment variables?
  > >  If argc and argv are passed to `QApplication`, you already know how 
it'll be parsed.
  >
  >
  > Because I don't see how I could add something to argv without leaking 
memory and potentially crash. I consider it as dangerous to modify the argv 
array and adding elements to it. It's something I do not dare.
  
  
  You don't modify it, you create a new one.
  
    new_argv = new char*[argc+2];
    new_argv[0] = argv[0];
    new_argv[1] = "-platform";
    new_argv[2] = "foo";
    for(int i = 1...argc) new_argv[i+2] = argv[i];
  
  Yes, you deliberately have to "leak" argc*sizeof(char*), but IMO it's more 
than worth not having to undo a very opaque hack everywhere where a child 
process could spawn.
  
  Please move the `qunsetenv` into a method in `KWorkSpace` at least.

REPOSITORY
  R120 Plasma Workspace

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

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

Reply via email to