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

INLINE COMMENTS

> filehelper.cpp:39
> +    }
> +    errno = -1;
> +}

this basically abuses a global variable for something that could just be a 
return value from this function.

> filehelper.cpp:44
> +{
> +    errno = 0;
> +

should be unnecessary now, with the above suggested change

> filehelper.cpp:134
> +
> +    if (errno)
> +        reply.setError(errno);

I hope we never have the opposite problem, where a libc function returns 
non-zero but "forgets" to set errno.
Now that you added return statements on success everywhere, maybe this line 
could say Q_ASSERT(errno != 0), or

  reply.setError(errno ? errno : -1);

to make sure we never return success when an error happened.

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

To: chinmoyr, elvisangelaccio, #frameworks, dfaure

Reply via email to