You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by SolidWallOfCode <gi...@git.apache.org> on 2016/10/14 13:51:56 UTC
[GitHub] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
GitHub user SolidWallOfCode opened a pull request:
https://github.com/apache/trafficserver/pull/1110
TS-4971: Change TSPluginRegistration to be const.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SolidWallOfCode/trafficserver ts-4971
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1110.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 #1110
----
commit d36efa65909f7c48ae84cc719eb3d7d788e90e95
Author: Alan M. Carroll <so...@yahoo-inc.com>
Date: 2016-10-14T13:50:39Z
TS-4971: Change TSPluginRegistration to be const.
----
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/917/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1110
I'll fix the `const` thing and do the plugin updates in a new PR. I tested the build and the plugins built fine, as expected.
---
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] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1110#discussion_r83442185
--- Diff: lib/ts/apidefs.h.in ---
@@ -105,9 +105,9 @@ extern "C" {
*/
typedef struct {
- char *plugin_name;
- char *vendor_name;
- char *support_email;
+ char const *plugin_name;
+ char const *vendor_name;
+ char const *support_email;
--- End diff --
Everything else in this file uses ``const char *``. It would be better to keep a consistent style.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1015/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1029/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1025/ for details.
---
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] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1110#discussion_r83534537
--- Diff: plugins/background_fetch/background_fetch.cc ---
@@ -551,13 +551,12 @@ void
TSPluginInit(int argc, const char *argv[])
{
TSPluginRegistrationInfo info;
- static const struct option longopt[] = {{const_cast<char *>("log"), required_argument, NULL, 'l'},
- {const_cast<char *>("config"), required_argument, NULL, 'c'},
- {NULL, no_argument, NULL, '\0'}};
+ static const struct option longopt[] = {
+ {"log", required_argument, NULL, 'l'}, {"config", required_argument, NULL, 'c'}, {NULL, no_argument, NULL, '\0'}};
--- End diff --
Solaris again :(
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1110
I think we should fix up the casts, or not. I think it's worse to say we'll fix some of the pointless casts but not others. If the goal is to show correct code, they should all be fixed. I also think if we're fixing this all C style casts should be removed. Those are strongly deprecated for C++ code. The point of the distinct casts is to make it clear what is being done.
I'm fine with splitting this into a PR for each commit. That's why I left them separated.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1010/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/907/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1024/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1026/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/902/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/918/ for details.
---
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] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode closed the pull request at:
https://github.com/apache/trafficserver/pull/1110
---
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] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1110#discussion_r83534567
--- Diff: plugins/esi/fetcher/HttpDataFetcherImpl.cc ---
@@ -80,7 +80,7 @@ HttpDataFetcherImpl::addFetchRequest(const string &url, FetchedDataProcessor *ca
if (length < (int)sizeof(buff)) {
http_req = buff;
} else {
- http_req = (char *)malloc(length + 1);
+ http_req = static_cast<char *>(malloc(length + 1));
--- End diff --
I'm not convinced by ``static_cast`` on ``malloc()``. It does nothing but look uglier. It would be better not to include so many different changes in this PR.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/919/ for details.
---
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] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1110#discussion_r83534526
--- Diff: plugins/authproxy/authproxy.cc ---
@@ -682,10 +682,10 @@ AuthParseOptions(int argc, const char **argv)
// on some platforms (e.g. Solaris / Illumos). On sane platforms (e.g. linux), it'll get
// automatically casted back to the const char*, as the struct is defined in <getopt.h>.
static const struct option longopt[] = {
- {const_cast<char *>("auth-host"), required_argument, 0, 'h'},
- {const_cast<char *>("auth-port"), required_argument, 0, 'p'},
- {const_cast<char *>("auth-transform"), required_argument, 0, 't'},
- {const_cast<char *>("force-cacheability"), no_argument, 0, 'c'},
+ {"auth-host", required_argument, 0, 'h'},
+ {"auth-port", required_argument, 0, 'p'},
+ {"auth-transform", required_argument, 0, 't'},
+ {"force-cacheability", no_argument, 0, 'c'},
--- End diff --
This change will break Solaris.
---
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] trafficserver pull request #1110: TS-4971: Change TSPluginRegistration to be...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1110#discussion_r83534541
--- Diff: plugins/esi/esi.cc ---
@@ -1599,12 +1599,9 @@ esiPluginInit(int argc, const char *argv[], struct OptionInfo *pOptionInfo)
if (argc > 1) {
int c;
static const struct option longopts[] = {
- {const_cast<char *>("packed-node-support"), no_argument, NULL, 'n'},
- {const_cast<char *>("private-response"), no_argument, NULL, 'p'},
- {const_cast<char *>("disable-gzip-output"), no_argument, NULL, 'z'},
- {const_cast<char *>("first-byte-flush"), no_argument, NULL, 'b'},
- {const_cast<char *>("handler-filename"), required_argument, NULL, 'f'},
- {NULL, 0, NULL, 0},
+ {"packed-node-support", no_argument, NULL, 'n'}, {"private-response", no_argument, NULL, 'p'},
+ {"disable-gzip-output", no_argument, NULL, 'z'}, {"first-byte-flush", no_argument, NULL, 'b'},
+ {"handler-filename", required_argument, NULL, 'f'}, {NULL, 0, NULL, 0},
--- End diff --
Solaris
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/921/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1027/ for details.
---
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] trafficserver issue #1110: TS-4971: Change TSPluginRegistration to be const.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1110
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/916/ for details.
---
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.
---