D6829: Add ability to use the new kauth helper in file ioslave

[David Faure]

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

INLINE COMMENTS

> global.h:270
> +    OperationAllowed,
> +    OperationCancelled
> +};

Canceled and cancelled as both correct English (maybe one is UK and one is US, I don't know).

In KIO we used Canceled everywhere, though. I would prefer if this stayed consistent with the rest of KIO..

> slavebase.h:944
> +     */
> +    int isPrivilegeOperationAllowed();
> +

If it returns an int, it can't really be named "isSomething" anymore, which is for boolean methods.

"Is this allowed?" "2" doesn't really work.

Can't this return an enum type rather than an int?
Also, the method should be const...

> file_p.h:24
>  
> +#define EUSERCANCELLED 255
> +

#define is for 1990, these days we have a proper C++ language where preprocessor hacks are less and less needed.

An enum value is probably the cleanest way here (to avoid the whole issue of "in which .cpp file to implement it", if it was an actual int variable).

The naming EFOO is very libc-like, I wouldn't use this here.
In fact, why not use KIO::ERR_USER_CANCELED?  (note that it has a value of 1, don't use 1 for something else ;)

> file_unix.cpp:666
> +        if (fileOpStatus == KIO::OperationCancelled) {
> +            errno = EUSERCANCELED;
> +        }

Who's going to read that value? The caller? Abusing errno to ship a return value via global state reads horrible to me.... we're not a libc function....

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

To: chinmoyr, dfaure, #frameworks
Cc: #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170815/fcb1e392/attachment-0001.html>