Re: Review Request 129729: [OS X] : don't risk deleting /Applications (!)

[René J . V . Bertin]

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

(Updated Jan. 8, 2017, 9:12 p.m.)


Status
--

This change has been discarded.


Review request for KDE Software on Mac OS X and KDE Frameworks.


Repository: kservice


Description
---

I noticed that kbuildsyscoca.cpp may delete 
`QStandardPaths::writableLocation(QStandardPaths::ApplicationsLocation)`, 
according to the comments if that location is empty, but in practice no check 
is made whether that's the case.

And that makes the tool very dangerous on Mac, where the native 
ApplicationsLocation is something very different.

This patch refuses to remove /Applications, and also checks if `appsDir` is 
indeed empty.


Diffs
-

  src/sycoca/kbuildsycoca.cpp 5e63907 

Diff: https://git.reviewboard.kde.org/r/129729/diff/


Testing
---

Works as intended (though I haven't tried to get it to fail, for some curious 
reason :))


Thanks,

René J.V. Bertin

Re: Review Request 129729: [OS X] : don't risk deleting /Applications (!)

[David Faure]

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




src/sycoca/kbuildsycoca.cpp (line 495)


QDir::remove only works on empty directories.

This will NOT do a recursive deletion, like QDir::removeRecursively() would.

This patch serves no purpose, other than protecting against the deletion of 
an *empty* /Applications directory -- which BTW should never be returned by 
*writableLocation*().


- David Faure


On Dec. 30, 2016, 7:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129729/
> ---
> 
> (Updated Dec. 30, 2016, 7:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> I noticed that kbuildsyscoca.cpp may delete 
> `QStandardPaths::writableLocation(QStandardPaths::ApplicationsLocation)`, 
> according to the comments if that location is empty, but in practice no check 
> is made whether that's the case.
> 
> And that makes the tool very dangerous on Mac, where the native 
> ApplicationsLocation is something very different.
> 
> This patch refuses to remove /Applications, and also checks if `appsDir` is 
> indeed empty.
> 
> 
> Diffs
> -
> 
>   src/sycoca/kbuildsycoca.cpp 5e63907 
> 
> Diff: https://git.reviewboard.kde.org/r/129729/diff/
> 
> 
> Testing
> ---
> 
> Works as intended (though I haven't tried to get it to fail, for some curious 
> reason :))
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Review Request 129729: [OS X] : don't risk deleting /Applications (!)

[René J . V . Bertin]

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

Review request for KDE Software on Mac OS X and KDE Frameworks.


Repository: kservice


Description
---

I noticed that kbuildsyscoca.cpp may delete 
`QStandardPaths::writableLocation(QStandardPaths::ApplicationsLocation)`, 
according to the comments if that location is empty, but in practice no check 
is made whether that's the case.

And that makes the tool very dangerous on Mac, where the native 
ApplicationsLocation is something very different.


Diffs
-

  src/sycoca/kbuildsycoca.cpp 5e63907 

Diff: https://git.reviewboard.kde.org/r/129729/diff/


Testing
---

Works as intended (though I haven't tried to get it to fail, for some curious 
reason :))


Thanks,

René J.V. Bertin