Differential D25443 Diff 80187 src/core/chmodjob.cpp

Changeset View

Standalone View

View Options

src/core/chmodjob.cpp

Show All 18 Lines1/* This file is part of the KDE libraries
19 Boston, MA 02110-1301, USA. 19​ Boston, MA 02110-1301, USA.
20*/ 20​*/
21 21​
22#include "chmodjob.h" 22​#include "chmodjob.h"
23#include "../pathhelpers_p.h" 23​#include "../pathhelpers_p.h"
24 24​
25#include <KLocalizedString> 25​#include <KLocalizedString>
26#include <KUser> 26​#include <KUser>
27#include <QLinkedList> ​
28#include <QDebug> 27​#include <QDebug>
29 28​
30 29​
31#include "listjob.h" 30​#include "listjob.h"
32#include "job_p.h" 31​#include "job_p.h"
33#include "jobuidelegatefactory.h" 32​#include "jobuidelegatefactory.h"
34#include "kioglobal_p.h" 33​#include "kioglobal_p.h"
35 34​
35​#include <stack>
36​
36namespace KIO 37​namespace KIO
37{ 38​{
38 39​
39struct ChmodInfo { 40​struct ChmodInfo {
40 QUrl url; 41​ QUrl url;
41 int permissions; 42​ int permissions;
42}; 43​};
43 44​
Show All 21 Lines52public:
65 ChmodJobState state; 66​ ChmodJobState state;
66 int m_permissions; 67​ int m_permissions;
67 int m_mask; 68​ int m_mask;
68 KUserId m_newOwner; 69​ KUserId m_newOwner;
69 KGroupId m_newGroup; 70​ KGroupId m_newGroup;
70 bool m_recursive; 71​ bool m_recursive;
71 bool m_bAutoSkipFiles; 72​ bool m_bAutoSkipFiles;
72 KFileItemList m_lstItems; 73​ KFileItemList m_lstItems;
73 QLinkedList<ChmodInfo> m_infos; // linkedlist since we keep removing the first item 74​ std::stack<ChmodInfo> m_infos;
74 75​
75 void _k_chmodNextFile(); 76​ void _k_chmodNextFile();
76 void _k_slotEntries(KIO::Job *, const KIO::UDSEntryList &); 77​ void _k_slotEntries(KIO::Job *, const KIO::UDSEntryList &);
77 void _k_processList(); 78​ void _k_processList();
78 79​
79 Q_DECLARE_PUBLIC(ChmodJob) 80​ Q_DECLARE_PUBLIC(ChmodJob)
80 81​
81 static inline ChmodJob *newJob(const KFileItemList &lstItems, int permissions, int mask, 82​ static inline ChmodJob *newJob(const KFileItemList &lstItems, int permissions, int mask,
Show All 24 Lines
106} 107​}
107 108​
108ChmodJob::~ChmodJob() 109​ChmodJob::~ChmodJob()
109{ 110​{
110} 111​}
111 112​
112void ChmodJobPrivate::_k_processList() 113​void ChmodJobPrivate::_k_processList()
113{ 114​{
114 Q_Q(ChmodJob); 115​ Q_Q(ChmodJob);
brunsUnsubmitted
Not Done

For efficiency, probably do a
m_infos.reserve(m_infos.capacity() + m_lstItems.size());
here.

bruns: For efficiency, probably do a `m_infos.reserve(m_infos.capacity() + m_lstItems.size());` here.
nicolasfellaAuthorUnsubmitted
Not Done

Does that make sense with a linked list?

nicolasfella: Does that make sense with a linked list?
115 while (!m_lstItems.isEmpty()) { 116​ while (!m_lstItems.isEmpty()) {
116 const KFileItem item = m_lstItems.first(); 117​ const KFileItem item = m_lstItems.first();
117 if (!item.isLink()) { // don't do anything with symlinks 118​ if (!item.isLink()) { // don't do anything with symlinks
118 // File or directory -> remember to chmod 119​ // File or directory -> remember to chmod
119 ChmodInfo info; 120​ ChmodInfo info;
120 info.url = item.url(); 121​ info.url = item.url();
121 // This is a toplevel file, we apply changes directly (no +X emulation here) 122​ // This is a toplevel file, we apply changes directly (no +X emulation here)
122 const mode_t permissions = item.permissions() & 0777; // get rid of "set gid" and other special flags 123​ const mode_t permissions = item.permissions() & 0777; // get rid of "set gid" and other special flags
123 info.permissions = (m_permissions & m_mask) | (permissions & ~m_mask); 124​ info.permissions = (m_permissions & m_mask) | (permissions & ~m_mask);
124 /*//qDebug() << "toplevel url:" << info.url << "\n current permissions=" << QString::number(permissions,8) 125​ /*//qDebug() << "toplevel url:" << info.url << "\n current permissions=" << QString::number(permissions,8)
125 << "\n wanted permission=" << QString::number(m_permissions,8) 126​ << "\n wanted permission=" << QString::number(m_permissions,8)
126 << "\n with mask=" << QString::number(m_mask,8) 127​ << "\n with mask=" << QString::number(m_mask,8)
127 << "\n with ~mask (mask bits we keep) =" << QString::number((uint)~m_mask,8) 128​ << "\n with ~mask (mask bits we keep) =" << QString::number((uint)~m_mask,8)
128 << "\n bits we keep =" << QString::number(permissions & ~m_mask,8) 129​ << "\n bits we keep =" << QString::number(permissions & ~m_mask,8)
129 << "\n new permissions = " << QString::number(info.permissions,8);*/ 130​ << "\n new permissions = " << QString::number(info.permissions,8);*/
130 m_infos.prepend(info); 131​ m_infos.push(std::move(info));
dfaureUnsubmitted
Done

This could even be push_front(std::move(info)) since info isn't used anymore afterwards;

dfaure: This could even be `push_front(std::move(info))` since `info` isn't used anymore afterwards;
131 //qDebug() << "processList : Adding info for " << info.url; 132​ //qDebug() << "processList : Adding info for " << info.url;
132 // Directory and recursive -> list 133​ // Directory and recursive -> list
133 if (item.isDir() && m_recursive) { 134​ if (item.isDir() && m_recursive) {
134 //qDebug() << "ChmodJob::processList dir -> listing"; 135​ //qDebug() << "ChmodJob::processList dir -> listing";
135 KIO::ListJob *listJob = KIO::listRecursive(item.url(), KIO::HideProgressInfo); 136​ KIO::ListJob *listJob = KIO::listRecursive(item.url(), KIO::HideProgressInfo);
136 q->connect(listJob, SIGNAL(entries(KIO::Job*,KIO::UDSEntryList)), 137​ q->connect(listJob, SIGNAL(entries(KIO::Job*,KIO::UDSEntryList)),
137 SLOT(_k_slotEntries(KIO::Job*,KIO::UDSEntryList))); 138​ SLOT(_k_slotEntries(KIO::Job*,KIO::UDSEntryList)));
138 q->addSubjob(listJob); 139​ q->addSubjob(listJob);
139 return; // we'll come back later, when this one's finished 140​ return; // we'll come back later, when this one's finished
140 } 141​ }
141 } 142​ }
142 m_lstItems.removeFirst(); 143​ m_lstItems.removeFirst();
143 } 144​ }
144 //qDebug() << "ChmodJob::processList -> going to STATE_CHMODING"; 145​ //qDebug() << "ChmodJob::processList -> going to STATE_CHMODING";
145 // We have finished, move on 146​ // We have finished, move on
146 state = CHMODJOB_STATE_CHMODING; 147​ state = CHMODJOB_STATE_CHMODING;
147 _k_chmodNextFile(); 148​ _k_chmodNextFile();
148} 149​}
149 150​
150void ChmodJobPrivate::_k_slotEntries(KIO::Job *, const KIO::UDSEntryList &list) 151​void ChmodJobPrivate::_k_slotEntries(KIO::Job *, const KIO::UDSEntryList &list)
151{ 152​{
152 KIO::UDSEntryList::ConstIterator it = list.begin(); 153​ KIO::UDSEntryList::ConstIterator it = list.begin();
153 KIO::UDSEntryList::ConstIterator end = list.end(); 154​ KIO::UDSEntryList::ConstIterator end = list.end();
154 for (; it != end; ++it) { 155​ for (; it != end; ++it) {
brunsUnsubmitted
Not Done

For efficiency, probably do a
m_infos.reserve(m_infos.capacity() + list.size());
here.

bruns: For efficiency, probably do a `m_infos.reserve(m_infos.capacity() + list.size());` here.
155 const KIO::UDSEntry &entry = *it; 156​ const KIO::UDSEntry &entry = *it;
156 const bool isLink = !entry.stringValue(KIO::UDSEntry::UDS_LINK_DEST).isEmpty(); 157​ const bool isLink = !entry.stringValue(KIO::UDSEntry::UDS_LINK_DEST).isEmpty();
157 const QString relativePath = entry.stringValue(KIO::UDSEntry::UDS_NAME); 158​ const QString relativePath = entry.stringValue(KIO::UDSEntry::UDS_NAME);
158 if (!isLink && relativePath != QLatin1String("..")) { 159​ if (!isLink && relativePath != QLatin1String("..")) {
159 const mode_t permissions = entry.numberValue(KIO::UDSEntry::UDS_ACCESS) 160​ const mode_t permissions = entry.numberValue(KIO::UDSEntry::UDS_ACCESS)
160 & 0777; // get rid of "set gid" and other special flags 161​ & 0777; // get rid of "set gid" and other special flags
161 162​
162 ChmodInfo info; 163​ ChmodInfo info;
Show All 16 Lines
179 } 180​ }
180 info.permissions = (m_permissions & mask) | (permissions & ~mask); 181​ info.permissions = (m_permissions & mask) | (permissions & ~mask);
181 /*//qDebug() << info.url << "\n current permissions=" << QString::number(permissions,8) 182​ /*//qDebug() << info.url << "\n current permissions=" << QString::number(permissions,8)
182 << "\n wanted permission=" << QString::number(m_permissions,8) 183​ << "\n wanted permission=" << QString::number(m_permissions,8)
183 << "\n with mask=" << QString::number(mask,8) 184​ << "\n with mask=" << QString::number(mask,8)
184 << "\n with ~mask (mask bits we keep) =" << QString::number((uint)~mask,8) 185​ << "\n with ~mask (mask bits we keep) =" << QString::number((uint)~mask,8)
185 << "\n bits we keep =" << QString::number(permissions & ~mask,8) 186​ << "\n bits we keep =" << QString::number(permissions & ~mask,8)
186 << "\n new permissions = " << QString::number(info.permissions,8);*/ 187​ << "\n new permissions = " << QString::number(info.permissions,8);*/
187 // Prepend this info in our todo list. 188​ // Push this info on top of the stack so it's handled first.
dfaureUnsubmitted
Done

"Prepend" is confusing now since push is more of an append. This comment needs an update, like "Push this info on top of the stack so it's handled first"

dfaure: "Prepend" is confusing now since push is more of an append. This comment needs an update, like…
188 // This way, the toplevel dirs are done last. 189​ // This way, the toplevel dirs are done last.
189 m_infos.prepend(info); 190​ m_infos.push(std::move(info));
dfaureUnsubmitted
Done

same here

dfaure: same here
190 } 191​ }
191 } 192​ }
192} 193​}
193 194​
194void ChmodJobPrivate::_k_chmodNextFile() 195​void ChmodJobPrivate::_k_chmodNextFile()
195{ 196​{
196 Q_Q(ChmodJob); 197​ Q_Q(ChmodJob);
197 if (!m_infos.isEmpty()) { 198​ if (!m_infos.empty()) {
198 ChmodInfo info = m_infos.takeFirst(); 199​ ChmodInfo info = m_infos.top();
200​ m_infos.pop();
199 // First update group / owner (if local file) 201​ // First update group / owner (if local file)
200 // (permissions have to set after, in case of suid and sgid) 202​ // (permissions have to set after, in case of suid and sgid)
201 if (info.url.isLocalFile() && (m_newOwner.isValid() || m_newGroup.isValid())) { 203​ if (info.url.isLocalFile() && (m_newOwner.isValid() || m_newGroup.isValid())) {
202 QString path = info.url.toLocalFile(); 204​ QString path = info.url.toLocalFile();
203 if (!KIOPrivate::changeOwnership(path, m_newOwner, m_newGroup)) { 205​ if (!KIOPrivate::changeOwnership(path, m_newOwner, m_newGroup)) {
204 if (!m_uiDelegateExtension) { 206​ if (!m_uiDelegateExtension) {
205 emit q->warning(q, i18n("Could not modify the ownership of file %1", path)); 207​ emit q->warning(q, i18n("Could not modify the ownership of file %1", path));
206 } else if (!m_bAutoSkipFiles) { 208​ } else if (!m_bAutoSkipFiles) {
207 const QString errMsg = i18n("<qt>Could not modify the ownership of file <b>%1</b>. You have insufficient access to the file to perform the change.</qt>", path); 209​ const QString errMsg = i18n("<qt>Could not modify the ownership of file <b>%1</b>. You have insufficient access to the file to perform the change.</qt>", path);
208 SkipDialog_Options options; 210​ SkipDialog_Options options;
209 if (m_infos.count() > 1) { 211​ if (m_infos.size() > 1) {
210 options |= SkipDialog_MultipleItems; 212​ options |= SkipDialog_MultipleItems;
211 } 213​ }
212 const SkipDialog_Result skipResult = m_uiDelegateExtension->askSkip(q, options, errMsg); 214​ const SkipDialog_Result skipResult = m_uiDelegateExtension->askSkip(q, options, errMsg);
213 switch (skipResult) { 215​ switch (skipResult) {
214 case Result_AutoSkip: 216​ case Result_AutoSkip:
215 m_bAutoSkipFiles = true; 217​ m_bAutoSkipFiles = true;
216 // fall through 218​ // fall through
217 Q_FALLTHROUGH(); 219​ Q_FALLTHROUGH();
218 case Result_Skip: 220​ case Result_Skip:
219 QMetaObject::invokeMethod(q, "_k_chmodNextFile", Qt::QueuedConnection); 221​ QMetaObject::invokeMethod(q, "_k_chmodNextFile", Qt::QueuedConnection);
220 return; 222​ return;
221 case Result_Retry: 223​ case Result_Retry:
222 m_infos.prepend(info); 224​ m_infos.push(std::move(info));
dfaureUnsubmitted
Done

same here

dfaure: same here
223 QMetaObject::invokeMethod(q, "_k_chmodNextFile", Qt::QueuedConnection); 225​ QMetaObject::invokeMethod(q, "_k_chmodNextFile", Qt::QueuedConnection);
224 return; 226​ return;
225 case Result_Cancel: 227​ case Result_Cancel:
226 default: 228​ default:
227 q->setError(ERR_USER_CANCELED); 229​ q->setError(ERR_USER_CANCELED);
228 q->emitResult(); 230​ q->emitResult();
229 return; 231​ return;
230 } 232​ }
▲ Show 20 Lines • Show All 63 Lines • Show Last 20 Lines