You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Alexander Shigin (JIRA)" <ji...@apache.org> on 2008/07/11 11:16:31 UTC
[jira] Created: (THRIFT-77) Remove fprintf from C++ lib
Remove fprintf from C++ lib
---------------------------
Key: THRIFT-77
URL: https://issues.apache.org/jira/browse/THRIFT-77
Project: Thrift
Issue Type: Improvement
Components: Library (C++)
Reporter: Alexander Shigin
Priority: Minor
Attachments: thrift-lib-cpp-remove-printf.patch
The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
2. There is a lot similar lines in lib/cpp:
{code}
string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
GlobalOutput(errStr.c_str());
{code}
The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-77) Remove fprintf from C++ lib
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613190#action_12613190 ]
Alexander Shigin commented on THRIFT-77:
----------------------------------------
Ooops, sorry. I have no objection.
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-77) Remove fprintf from C++ lib
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612967#action_12612967 ]
David Reiss commented on THRIFT-77:
-----------------------------------
I want to make a few tweaks to this patch, but I can't get to apply. Can you email your ssh public key to the list so we can give you an account on thrift-rpc.org and you can push your branch? Instructions here: <http://wiki.apache.org/thrift/GitRepository>.
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-77) Remove fprintf from C++ lib
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612986#action_12612986 ]
Alexander Shigin commented on THRIFT-77:
----------------------------------------
Sorry, I can't send keys right now.
Can you pull from git://github.com/shigin/thrift.git remove-fprintf branch? I've just resolved a config.
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-77) Remove fprintf from C++ lib
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613133#action_12613133 ]
David Reiss commented on THRIFT-77:
-----------------------------------
This is a big improvement in code, quality. Thanks for submitting it. I've made a few tweaks.
My full diff: http://gitweb.thrift-rpc.org/?p=thrift.git;a=treediff;h=6373742;hp=fb1007d;hb=6373742;hpb=fb1007d
Diff from original to mine: http://gitweb.thrift-rpc.org/?p=thrift.git;a=commitdiff;h=6373742
I fixed a few bugs in TOuput::printf, added a few comments, and restored errno_copy in a few places where the string constructor could possibly blow away errno by allocating memory.
Thoughts?
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-77) Remove fprintf from C++ lib
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alexander Shigin updated THRIFT-77:
-----------------------------------
Attachment: thrift-lib-cpp-remove-printf.patch
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-77) Remove fprintf from C++ lib
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-77.
-------------------------------
Resolution: Fixed
Assignee: Alexander Shigin
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Assignee: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-77) Remove fprintf from C++ lib
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613153#action_12613153 ]
Alexander Shigin commented on THRIFT-77:
----------------------------------------
I'm agreed with most changes, except one.
You've converted stack_buf to static variable. I can be wrong, but this change makes TOutput.printf thread unsafe.
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-77) Remove fprintf from C++ lib
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613184#action_12613184 ]
David Reiss commented on THRIFT-77:
-----------------------------------
{noformat}
+ static const int STACK_BUF_SIZE = 256;
+ char stack_buf[STACK_BUF_SIZE];
{noformat}
I'm pretty sure only the size is static.
> Remove fprintf from C++ lib
> ---------------------------
>
> Key: THRIFT-77
> URL: https://issues.apache.org/jira/browse/THRIFT-77
> Project: Thrift
> Issue Type: Improvement
> Components: Library (C++)
> Reporter: Alexander Shigin
> Priority: Minor
> Attachments: thrift-lib-cpp-remove-printf.patch
>
>
> The patch introduces two new methods for TOutput: perror and printf. These two methods are needed because:
> 1. TNonblocking has a couple of "fprintf(stderr, ""...);" clauses.
> 2. There is a lot similar lines in lib/cpp:
> {code}
> string errStr = "TNonblockingServer::Task: close, possible resource leak " + TOutput::strerror_s(errno);
> GlobalOutput(errStr.c_str());
> {code}
> The patch also uses the introduced methods to fix all these issues.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.