Improve error handling in file ioslave
ClosedPublic
Actions

Authored by chinmoyr on Feb 3 2018, 9:10 AM.

Details

Reviewers

Commits

R241:27aa3764c0bf: Improve error handling in file ioslave

Summary

When executing an action with elevated privileges file ioslave directly checks errno
and then decides whether to continue or not. Since errno can change frequently
between function call it should not be relied upon.
With this patch after a function fails errno value is saved and then that saved value
is used wherever required.

Diff Detail

Repository

R241 KIO

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

chinmoyr created this revision.Feb 3 2018, 9:10 AM

Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 3 2018, 9:10 AM

Restricted Application added a subscriber: Frameworks. · View Herald Transcript

chinmoyr requested review of this revision.Feb 3 2018, 9:10 AM

Used KIO error codes for windows

dfaure requested changes to this revision.Feb 3 2018, 9:31 AM

dfaure added inline comments.

src/ioslaves/file/file_unix.cpp
122	Why errno here? What sets it? Shouldn't that be errcode?
815	What happened to the code that was passing socketPath() for OPEN and OPENDIR? It wasn't needed?
src/ioslaves/file/file_win.cpp
392	Why not just return the errcode that was given as input? Won't this change mess up all error handling on Windows, otherwise, by "eating" the proper error codes?

This revision now requires changes to proceed.Feb 3 2018, 9:31 AM

errno -> errcode
KIO::ERR_ACCESS_DENIED -> err
socketPath is passed as an argument in line 106

socketPath() being passed by argument: true for OPEN indeed, but what about OPENDIR?

In D10266#199961, @dfaure wrote:

socketPath() being passed by argument: true for OPEN indeed, but what about OPENDIR?

No method in FileProtocol calls execWithElevatedPrivilege with OPENDIR action. If one does in future then we can pass socetPath() as argument there.

Ah OK so it was dead code. Removing dead code is always good.

The patch looks ok to me now -- assuming the unittests still pass, which you didn't mention in the testing section of this review request.

This revision is now accepted and ready to land.Feb 3 2018, 10:27 AM

The unittests pass.

Closed by commit R241:27aa3764c0bf: Improve error handling in file ioslave (authored by chinmoyr). · Explain WhyFeb 3 2018, 10:36 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/ioslaves/file/file.h (9 lines)
M			src/ioslaves/file/file.cpp (28 lines)
M			src/ioslaves/file/file_p.h (17 lines)
M			src/ioslaves/file/file_unix.cpp (92 lines)
M			src/ioslaves/file/file_win.cpp (13 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	26412	c4b1a28		Feb 3 2018, 9:09 AM	★	★
Diff 2	26415	c4b1a28	Used KIO error codes for windows	Feb 3 2018, 9:21 AM	★	★
Diff 3	26417	c4b1a28	errno -> errcode	Feb 3 2018, 9:40 AM	★	★
Diff 4	26420	658b38f	R241:27aa3764c0bf5abf2c6c7529a1ba17930f89ea8c	Feb 3 2018, 10:36 AM	★	★