D6197: Add kauth helper to file ioslave

David Faure noreply at phabricator.kde.org
Sat Jul 29 06:12:42 UTC 2017


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

INLINE COMMENTS

> filehelper.cpp:71
> +        chmod(path.data(), mode);
> +    } else if (action == CHOWN) {
> +        int uid = arg2.toInt();

this calls for a switch()

> filehelper.cpp:86
> +    } else if(action == OPENDIR) {
> +        DIR *dp = opendir(path.data());
> +        int fd = dirfd(dp);

opendir can return 0, in which case the next line will probably crash.

Overall, this whole method seriously lacks error handling in my opinion. Just checking for errno at the end isn't enough, e.g. in cases like this where multiple calls are being made.

And even otherwise, I'm unsure if ignoring the return value of chmod/chown/unlink/mkdir and *just* checking errno is enough or not. Does anyone else know?

> filehelper.h:22
> +
> +#ifndef HELPER_H
> +#define HELPER_H

should be FILEHELPER_H to match the filename

> filehelper.h:33
> + *
> + * @since 5.37
> + */

This isn't public API -> no @since.

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/20170729/245d8b4f/attachment.html>


More information about the Kde-frameworks-devel mailing list