DevHeads.net

Review Request 112529: By default hide SMB shares that end with $

Review request for KDE Runtime.

Description
The attached patch marks all files, directories, shares etc as hidden by default. That what the GUI applications can correctly hide them and only make them visible when the "Show Hidden Files" option is checked.

This addresses bug 197903.
<a href="http://bugs.kde.org/show_bug.cgi?id=197903" title="http://bugs.kde.org/show_bug.cgi?id=197903">http://bugs.kde.org/show_bug.cgi?id=197903</a>

Diffs

Diff: <a href="http://git.reviewboard.kde.org/r/112529/diff/" title="http://git.reviewboard.kde.org/r/112529/diff/">http://git.reviewboard.kde.org/r/112529/diff/</a>

Testing

Thanks,

Dawit Alemayehu

Comments

Re: Review Request 112529: By default hide SMB shares that end w

By Dawit A at 09/05/2013 - 09:32

(Updated Sept. 5, 2013, 1:32 p.m.)

Review request for KDE Runtime.

Changes
Uploaded patch

Description
The attached patch marks all files, directories, shares etc as hidden by default. That what the GUI applications can correctly hide them and only make them visible when the "Show Hidden Files" option is checked.

This addresses bug 197903.
<a href="http://bugs.kde.org/show_bug.cgi?id=197903" title="http://bugs.kde.org/show_bug.cgi?id=197903">http://bugs.kde.org/show_bug.cgi?id=197903</a>

Diffs (updated)
kioslave/smb/kio_smb_browse.cpp fec6449

Diff: <a href="http://git.reviewboard.kde.org/r/112529/diff/" title="http://git.reviewboard.kde.org/r/112529/diff/">http://git.reviewboard.kde.org/r/112529/diff/</a>

Testing

Thanks,

Dawit Alemayehu

Re: Review Request 112529: By default hide SMB shares that end w

By Dawit A at 09/06/2013 - 08:35

(Updated Sept. 6, 2013, 12:35 p.m.)

Review request for KDE Runtime.

Changes
Updated patch.

Description
The attached patch marks all files, directories, shares etc as hidden by default. That what the GUI applications can correctly hide them and only make them visible when the "Show Hidden Files" option is checked.

This addresses bug 197903.
<a href="http://bugs.kde.org/show_bug.cgi?id=197903" title="http://bugs.kde.org/show_bug.cgi?id=197903">http://bugs.kde.org/show_bug.cgi?id=197903</a>

Diffs (updated)
kioslave/smb/kio_smb_browse.cpp fec6449

Diff: <a href="http://git.reviewboard.kde.org/r/112529/diff/" title="http://git.reviewboard.kde.org/r/112529/diff/">http://git.reviewboard.kde.org/r/112529/diff/</a>

Testing (updated)
Tested against shares from a Windows 7 machine.

Thanks,

Dawit Alemayehu

Re: Review Request 112529: By default hide SMB shares that end w

By Dawit A at 09/06/2013 - 09:04

(Updated Sept. 6, 2013, 1:04 p.m.)

Review request for KDE Runtime.

Changes
Updated patch

Description
The attached patch marks all files, directories, shares etc as hidden by default. That what the GUI applications can correctly hide them and only make them visible when the "Show Hidden Files" option is checked.

This addresses bug 197903.
<a href="http://bugs.kde.org/show_bug.cgi?id=197903" title="http://bugs.kde.org/show_bug.cgi?id=197903">http://bugs.kde.org/show_bug.cgi?id=197903</a>

Diffs (updated)
kioslave/smb/kio_smb_browse.cpp fec6449

Diff: <a href="http://git.reviewboard.kde.org/r/112529/diff/" title="http://git.reviewboard.kde.org/r/112529/diff/">http://git.reviewboard.kde.org/r/112529/diff/</a>

Testing
Tested against shares from a Windows 7 machine.

Thanks,

Dawit Alemayehu

Re: Review Request 112529: By default hide SMB shares that end w

By Commit Hook at 09/09/2013 - 08:29

This review has been submitted with commit b21dc0aae946b1fe66b189ecb4cf4447624156f2 by Dawit Alemayehu to branch KDE/4.11.

- Commit Hook

On Sept. 6, 2013, 1:04 p.m., Dawit Alemayehu wrote:

Re: Review Request 112529: By default hide SMB shares that end w

By Commit Hook at 09/09/2013 - 08:29

(Updated Sept. 9, 2013, 12:29 p.m.)

Status
This change has been marked as submitted.

Review request for KDE Runtime.

Description
The attached patch marks all files, directories, shares etc as hidden by default. That what the GUI applications can correctly hide them and only make them visible when the "Show Hidden Files" option is checked.

This addresses bug 197903.
<a href="http://bugs.kde.org/show_bug.cgi?id=197903" title="http://bugs.kde.org/show_bug.cgi?id=197903">http://bugs.kde.org/show_bug.cgi?id=197903</a>

Diffs
kioslave/smb/kio_smb_browse.cpp fec6449

Diff: <a href="http://git.reviewboard.kde.org/r/112529/diff/" title="http://git.reviewboard.kde.org/r/112529/diff/">http://git.reviewboard.kde.org/r/112529/diff/</a>

Testing
Tested against shares from a Windows 7 machine.

Thanks,

Dawit Alemayehu

Re: Review Request 112529: By default hide SMB shares that end w

By Mark at 09/06/2013 - 09:09

as far as i can judge it, it looks OK to me. Someone else would have to give a ship it.

- Mark Gaiser

On Sept. 6, 2013, 1:04 p.m., Dawit Alemayehu wrote:

Re: Review Request 112529: By default hide SMB shares that end w

By Mark at 09/06/2013 - 08:40

kioslave/smb/kio_smb_browse.cpp
<http://git.reviewboard.kde.org/r/112529/#comment29075>

You forgot to remove this one?

- Mark Gaiser

On Sept. 6, 2013, 12:35 p.m., Dawit Alemayehu wrote:

Re: Review Request 112529: By default hide SMB shares that end w

By Mark at 09/06/2013 - 07:04

kioslave/smb/kio_smb_browse.cpp
<http://git.reviewboard.kde.org/r/112529/#comment29069>

This - while most was there already - seems odd to me. Your other code catches IPC$, ADMIN$, printer$ and print$ (because of the "$") so i think you can trim this down to just "." and ".." and then just break out of the current loop interation.

kioslave/smb/kio_smb_browse.cpp
<http://git.reviewboard.kde.org/r/112529/#comment29068>

Move this code up above the if-elseif-elseif-.... Around the line: udsentry.insert( KIO::UDSEntry::UDS_NAME, udsName );

Doing that means that you only have to add this code once instead of 4 times (like you did now). You can remove the other ones.

kioslave/smb/kio_smb_browse.cpp
<http://git.reviewboard.kde.org/r/112529/#comment29070>

See other comment about the same code.

kioslave/smb/kio_smb_browse.cpp
<http://git.reviewboard.kde.org/r/112529/#comment29071>

See other comment about the same code.

kioslave/smb/kio_smb_browse.cpp
<http://git.reviewboard.kde.org/r/112529/#comment29072>

See other comment about the same code.

I don't know much in the samba department, but just happened to touch the exact same code a few days ago when porting it to Qt5/KF5 - which is still waiting for someone to review it btw ;)

- Mark Gaiser

On Sept. 5, 2013, 1:32 p.m., Dawit Alemayehu wrote:

Re: Review Request 112529: By default hide SMB shares that end w

By Dawit A at 09/06/2013 - 08:36

All of them were there already. I did not add any new ones. I simply moved them around. Anyhow, I completely misread what Windows administrative shares meant. I thought the drive letters that ended with $ were different. That is why I kept the existing code and did the patch the way I did it. Upon a second review all so called hidden shares, those that end with $, are administrative shares ; so I am going to change this to do exactly as you suggested above.

On Sept. 6, 2013, 11:04 a.m., Dawit Alemayehu wrote:
I saw that. I was going to comment on it but I forgot. I will do that now.

- Dawit

On Sept. 6, 2013, 12:35 p.m., Dawit Alemayehu wrote:

Re: Review Request 112529: By default hide SMB shares that end w

By Kai Uwe Broulik at 09/05/2013 - 05:02

There seems to be no patch attached

- Kai Uwe Broulik

On Sept. 5, 2013, 3:09 a.m., Dawit Alemayehu wrote: