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
