You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by jeking3 <gi...@git.apache.org> on 2016/06/22 16:17:40 UTC

[GitHub] thrift pull request #1033: THRIFT-2156: fix errno handling in server socket

GitHub user jeking3 opened a pull request:

    https://github.com/apache/thrift/pull/1033

    THRIFT-2156: fix errno handling in server socket

    I took the information from the patch submitted in 2013 and applied it to the code where appropriate to fix the errno handling.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jeking3/thrift defect/THRIFT-2156

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1033.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1033
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1033: THRIFT-2156: fix errno handling in server socket

Posted by tpcwang <gi...@git.apache.org>.
Github user tpcwang commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1033#discussion_r68087439
  
    --- Diff: lib/cpp/src/thrift/transport/TServerSocket.cpp ---
    @@ -520,23 +523,23 @@ void TServerSocket::listen() {
       if (retries > retryLimit_) {
         char errbuf[1024];
         if (!path_.empty()) {
    -      sprintf(errbuf, "TServerSocket::listen() PATH %s", path_.c_str());
    +      snprintf(errbuf, 1024, "TServerSocket::listen() PATH %s", path_.c_str());
    --- End diff --
    
    ditto here with sizeof(errbuf)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1033: THRIFT-2156: fix errno handling in server socket

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/thrift/pull/1033


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1033: THRIFT-2156: fix errno handling in server socket

Posted by tpcwang <gi...@git.apache.org>.
Github user tpcwang commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1033#discussion_r68087139
  
    --- Diff: lib/cpp/src/thrift/transport/TServerSocket.cpp ---
    @@ -284,7 +284,7 @@ void TServerSocket::listen() {
       hints.ai_family = PF_UNSPEC;
       hints.ai_socktype = SOCK_STREAM;
       hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
    -  sprintf(port, "%d", port_);
    +  snprintf(port, sizeof("65535"), "%d", port_);
    --- End diff --
    
    sizeof(port) seems ever-so-slightly clearer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1033: THRIFT-2156: fix errno handling in server socket

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1033#discussion_r68088614
  
    --- Diff: lib/cpp/src/thrift/transport/TServerSocket.cpp ---
    @@ -284,7 +284,7 @@ void TServerSocket::listen() {
       hints.ai_family = PF_UNSPEC;
       hints.ai_socktype = SOCK_STREAM;
       hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
    -  sprintf(port, "%d", port_);
    +  snprintf(port, sizeof("65535"), "%d", port_);
    --- End diff --
    
    I think the allocation of the buffer for port is just as bizarre, but I wanted to remain consistent.  Let's see if the build passes; if it fails and I need to rebase on master to kick another because of a transient issue, I can make the changes you suggest.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift issue #1033: THRIFT-2156: fix errno handling in server socket

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1033
  
    Rebased now that the correct fix for THRIFT-3768 is in which was causing build failures... using this to prove that is resolved so I can re-mark THRIFT-3768 closed and THRIFT-3861 fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift issue #1033: THRIFT-2156: fix errno handling in server socket

Posted by tpcwang <gi...@git.apache.org>.
Github user tpcwang commented on the issue:

    https://github.com/apache/thrift/pull/1033
  
    :+1: I don't really mind if you want to leave the two noted lines as-is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---