fvogt requested changes to this revision.
fvogt added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> fdreceiver.cpp:42
> + std::cerr << "Invalid socket address" << std::endl;
> + return;
> + }
Missing
::close(m_socketDes);
m_socketDes = -1;
or just move it above the `m_socketDes` assignment.
> fdreceiver.cpp:60
> }
> + ::unlink(m_path.toStdString().c_str());
> }
A quick question: Is it possible that there are two FdReceivers with the same
pid?
In that case they would end up removing each other's sockets.
> file_unix.cpp:91
> + // value and reset it after deleting socket file.
> + int err = errno;
> + QFile::remove(sockPath);
That looks like a hack. If errno is that important, save and keep it in a
variable. Every call into a library can change errno, it's extremely volatile.
> sharefd_p.h:53
> }
> + bool error() const
> + {
Just a suggestion: Remove `error()` and just return `nullptr` in `address()` in
the error case.
That way it's not possible to use it accidentially.
> sharefd_p.h:66
> + ::strncpy(a.sun_path, path.c_str(), sizeof(a.sun_path)-1);
> + }
> return a;
The socket permissions only work that way for linux, so if other OSs can use
this code as well it's insecure and should `#error` out.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D9966
To: chinmoyr, #frameworks, thiago, fvogt
Cc: ngraham, fvogt, lbeltrame, dfaure
