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
