D6197: Add kauth helper to file ioslave

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> chinmoyr wrote in filehelper.cpp:86
> In case of OPEN and OPENDIR, I can see why we need error checking, opendir() can return null and there are some issues (I read on internet, can't recall the source) if close() fails. But in case of single function calls like chmod() I can't see how return value matters. Say if unlink fails for whatever reasons, we can return the error code and the checking for exact error code can be done by ioslave. Is there something I am overlooking?

The documentation for chown and others says:

Upon successful completion, these functions shall return 0.  Otherwise, these functions shall return −1 and set errno to indicate the error.

It does NOT say, that errno isn't set when the function is successful. It seems to me that it would be perfectly valid for a libc implementation to do something like "try this, it fails (and sets errno), then try that, it worked, return 0".

For this reason I would feel much safer (especially in code run by root!) if the error handling was more classic, i.e. by checking return values.

> filehelper.cpp:41
> +
> +enum {
> +    CHMOD = 1,

I assume this enum is duplicated elsewhere (in whoever is serializing the arguments for FileHelper), right?
Can the duplication be avoided by sharing a private header? Otherwise this at least needs a comment like "keep in sync with..." in both places.

> filehelper.cpp:102
> +            closedir(dp);
> +        }
> +        sendFileDescriptor(-1);

missing else or break here, no?
You're sending the valid fd, followed by -1.

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

To: chinmoyr, elvisangelaccio, #frameworks, dfaure
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170803/7e5ba12d/attachment.html>