ulidtko commented on a change in pull request #2234:
URL: https://github.com/apache/thrift/pull/2234#discussion_r485650407
##########
File path: lib/cpp/src/thrift/transport/TServerSocket.cpp
##########
@@ -408,24 +407,21 @@ void TServerSocket::listen() {
}
struct sockaddr_un address;
+ // Store the zero-terminated path in address.sun_path
+ memset(&address, '\0', sizeof(address));
address.sun_family = AF_UNIX;
- memcpy(address.sun_path, path_.c_str(), len);
-
- auto structlen = static_cast<socklen_t>(sizeof(address));
+ memcpy(address.sun_path, path_.c_str(), path_.size());
if (!address.sun_path[0]) { // abstract namespace socket
-#ifdef __linux__
- // sun_path is not null-terminated in this case and structlen determines
its length
- structlen -= sizeof(address.sun_path) - len;
-#else
+#ifndef __linux__
GlobalOutput.perror("TSocket::open() Abstract Namespace Domain sockets
only supported on linux: ", -99);
throw TTransportException(TTransportException::NOT_OPEN,
" Abstract Namespace Domain socket path not
supported");
#endif
}
do {
- if (0 == ::bind(serverSocket_, (struct sockaddr*)&address, structlen)) {
+ if (0 == ::bind(serverSocket_, (struct sockaddr*)&address,
static_cast<socklen_t>(sizeof(address)))) {
Review comment:
I'm a bit wary about this `sizeof(address)` — because of how `struct
sockaddr` can have more data following it. What type is `address` again?..
`man 7 unix` says this:
> **Pathname sockets**
> When binding a socket to a pathname, a few rules should be observed
for maximum portability and ease of coding:
>
> * The pathname in sun_path should be null-terminated.
> * The length of the pathname, including the terminating null byte,
should not exceed the size of sun_path.
> * The addrlen argument that describes the enclosing sockaddr_un
structure should have a value of at least:
>
> offsetof(struct sockaddr_un, sun_path)+strlen(addr.sun_path)+1
>
> or, more simply, addrlen can be specified as `sizeof(struct sockaddr_un)`.
Ah, so I see, line 409, it's exactly `sockaddr_un`. So all's good.
How about maybe:
```suggestion
if (0 == ::bind(serverSocket_, (struct sockaddr*)&address,
static_cast<socklen_t>(sizeof(struct sockaddr_un)))) {
```
? And similarly below
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
