Hi Dhammika,
I've checked this patch. See my comments inlines.
Sorry for the delay :( I still have your shutdown patch in the review
queue. I've checked it once and it seemed to be OK, but the shutdown
code is so complex that I want to check it once more.
Martin
> From 79b05df46e454106fa63fbda435e8ca396a09063 Mon Sep 17 00:00:00 2001
> From: Dhammika Pathirana<dhamm...@gmail.com>
> Date: Thu, 9 Dec 2010 22:35:40 -0800
> Subject: [PATCH] fix unix domain socket addrlen
>
>
> Signed-off-by: Dhammika Pathirana<dhamm...@gmail.com>
> ---
> src/ip.cpp | 2 +-
> src/tcp_listener.cpp | 8 ++++----
> src/tcp_listener.hpp | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/ip.cpp b/src/ip.cpp
> index eb05aec..14744ab 100644
> --- a/src/ip.cpp
> +++ b/src/ip.cpp
> @@ -324,7 +324,7 @@ int zmq::resolve_local_path (sockaddr_storage
> *addr_, socklen_t *addr_len_,
> }
> strcpy (un->sun_path, path_);
> un->sun_family = AF_UNIX;
> - *addr_len_ = sizeof (sockaddr_un);
> + *addr_len_ = sizeof (un->sun_family) + strlen (path_) + 1;
Is it right to create address that is shorter than the stucture designed
to hold it?
And even if so, why is it necessary in the first place. If it works OK,
let's rather let is be so that we don't break the behaviour on some
exotic system.
> return 0;
> }
>
> diff --git a/src/tcp_listener.cpp b/src/tcp_listener.cpp
> index 6d41069..0693b01 100644
> --- a/src/tcp_listener.cpp
> +++ b/src/tcp_listener.cpp
> @@ -236,9 +236,9 @@ int zmq::tcp_listener_t::set_address (const char
> *protocol_, const char *addr_,
> errno_assert (rc != -1);
>
> // Bind the socket to the file path.
> - rc = bind (s, (struct sockaddr*)&addr, sizeof (sockaddr_un));
> + rc = bind (s, (struct sockaddr*)&addr, addr_len);
> if (rc != 0) {
> - close ();
> + close (false);
I would rather have a member variable, say "bool file_exists" that would
be set after the bind and used in the close function, rather than
passing it as an argument. What do you think?
> return -1;
> }
>
> @@ -258,7 +258,7 @@ int zmq::tcp_listener_t::set_address (const char
> *protocol_, const char *addr_,
> }
> }
>
> -int zmq::tcp_listener_t::close ()
> +int zmq::tcp_listener_t::close (bool unlink)
> {
> zmq_assert (s != retired_fd);
> int rc = ::close (s);
> @@ -270,7 +270,7 @@ int zmq::tcp_listener_t::close ()
> // If there's an underlying UNIX domain socket, get rid of the file it
> // is associated with.
> struct sockaddr_un *su = (struct sockaddr_un*)&addr;
> - if (AF_UNIX == su->sun_family) {
> + if (AF_UNIX == su->sun_family&& unlink) {
> rc = ::unlink(su->sun_path);
> if (rc != 0)
> return -1;
> diff --git a/src/tcp_listener.hpp b/src/tcp_listener.hpp
> index 1490a56..9107f10 100644
> --- a/src/tcp_listener.hpp
> +++ b/src/tcp_listener.hpp
> @@ -40,7 +40,7 @@ namespace zmq
> int backlog_);
>
> // Close the listening socket.
> - int close ();
> + int close (bool unlink = true);
>
> // Get the file descriptor to poll on to get notified about
> // newly created connections.
_______________________________________________
zeromq-dev mailing list
zeromq-dev@lists.zeromq.org
http://lists.zeromq.org/mailman/listinfo/zeromq-dev
