D10273: Create proper SocketAddress

[Oswald Buddenhagen]

ossi added inline comments.

INLINE COMMENTS

> fdreceiver.h:45
> +    QString m_path;
> +    QSocketNotifier *m_readNotifier;
>  };

why are you moving the member?
it doesn't matter in this case, but generally it's better to have the bigger members first, concentrating in particular on equal size (and sizeof(QString) == sizeof(void*)).
all other things being equal, prefer no change to reduce the diff size.

> sharefd_p.h:54
>  private:
> -    static sockaddr_un make_address(const std::string& path)
> +    static sockaddr_un make_address(const QByteArray& path)
>      {

space on wrong side of amp

> sharefd_p.h:60
> +        const size_t pathSize = path.size();
> +        if (pathSize > 0 && pathSize < sizeof(a.sun_path)-1) {
> +            ::strncpy(a.sun_path, path.constData(), sizeof(a.sun_path)-1);

put spaces around binary operators

> sharefd_p.h:60
> -        ::strcpy(a.sun_path, finalPath.c_str());
> -        ::unlink(finalPath.c_str());
> -#endif

you're removing that without replacement. that makes me think that the patch is still non-atomic.

> sharefd_p.h:61
> +        if (pathSize > 0 && pathSize < sizeof(a.sun_path)-1) {
> +            ::strncpy(a.sun_path, path.constData(), sizeof(a.sun_path)-1);
> +        }

using strncpy() after an explicit length check is relatively pointless (unless the trailing zero padding is important).
but the operation can be optimized by using memcpy().

REPOSITORY
  R241 KIO

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

To: chinmoyr, #frameworks
Cc: ossi, thiago, dfaure, michaelh, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180203/f3a79daf/attachment-0001.html>