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