You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by pbchou <gi...@git.apache.org> on 2016/04/14 21:16:19 UTC
[GitHub] trafficserver pull request: TS-3245 : Changes optind = 1 to optind...
GitHub user pbchou opened a pull request:
https://github.com/apache/trafficserver/pull/571
TS-3245 : Changes optind = 1 to optind = 0 for all plugins to ensure
that multiples plugins in the global plugin.config can co-exist
together. Note that all plugins were already optind = 0 except
for the four that were modified in the this commit (background_fetch,
regex_revalidate, remap_stats, and stale_while_revalidate).
Note: This patch does NOT change ATS core so that future plugins
must be sure to use optind = 0 also.
Understood preference is for ATS core change to cover all plugins, but this has been outstanding for more than one year and affects our usage of ATS. Also, I am not sure if an ATS core change can cover the plugins since according to the Linux man page setting optind=0 and calling getopt_long() causes an "internal" initialization process to run which looks at the optstring argument to getopt_long(). ATS core would not have pre-knowledge of each plugin's internals so this seems like this would be a road-block?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pbchou/trafficserver TS-3245
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/571.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 #571
----
commit a5141b2a90206d439841e1bd96aa5e8b15bcc9dd
Author: Peter Chou <pb...@labs.att.com>
Date: 2016-04-12T18:34:01Z
TS-3245 : Changes optind = 1 to optind = 0 for all plugins to ensure
that multiples plugins in the global plugin.config can co-exist
together. Note that all plugins were already optind = 0 except
for the four that were modified in the this commit (background_fetch,
regex_revalidate, remap_stats, and stale_while_revalidate).
Note: This patch does NOT change ATS core so that future plugins
must be sure to use optind = 0 also.
----
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219186502
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou closed the pull request at:
https://github.com/apache/trafficserver/pull/571
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219553317
Can you please explain "do not play well together"? The `:` makes that option required I think. Also, most if not all of these plugins are developed/tested/deployed with glibc.
My preference would be to do this in the core inside some sort of `__GNU_LIBRARY__` ifdef block. We do support FreeBSD and OpenSolaris.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219573746
* ``getopt_long`` is implemented on all the platforms we support.
* ``:`` means the option takes an argument; it is not a GNU extension
Here's what we should test http://apaste.info/5lw . If we detect that we are glibc and set ``optind`` to ``0`` that is fine too.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219127822
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219586815
Ok, well I assume @pbchou can take it from here?
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218904513
FreeBSD v10 build finished successfully. Details on https://ci.trafficserver.apache.org/job/Github-FreeBSD/16/
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218586966
[approve ci]
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219260120
@pbchou So a complication might be if we have platforms where "0" does not make sense? Or do we feel comfortable that "0" is what we should set? Do we have to #ifdef this?
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-214041833
Note to self: check what glibc does with ``optind=1``, which is the pattern used by remap plugins.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219485292
I can only speak for Linux. However, it should be noted that if you grep for 'optind = 0' under plugins/ there are around ten other plugins already using optind = 0 and none appear to have used ifdefs within close proximity (I used egrep -B5). I would recommend to first fix the four plugins covered by this patch since if there are cross-platform issues then that would require a much wider change that could then be applied uniformly across all 14 plugins.
---
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 #571: TS-3245 : Changes optind = 1 to optind = 0 ...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou closed the pull request at:
https://github.com/apache/trafficserver/pull/571
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218591913
Linux (CentOS7) build finished successfully. Details on https://ci.trafficserver.apache.org/job/Github-Linux/14/
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219535666
``0`` is a GNU extension behaviour, from the [man page](http://man7.org/linux/man-pages/man3/getopt.3.html):
```
... and wants to make use of GNU extensions such as '+' and '-' at the start
of optstring, or changes the value of POSIXLY_CORRECT between scans,
must reinitialize getopt() by resetting optind to 0, rather than the traditional
value of 1.
```
This is probably where the confusion originates, since for glibc either ``0`` or ``1`` should work, depending on which part of the man page the author read. On non-gllibc, ``1`` is the only value that would work. AFAIK we don't have any plugins that use GNU extensions, so I recommend we reset ``optind`` to ``1`` in the plugin loader so that plugins always get the right behaviour by default.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219128077
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218903158
Linux (CentOS7) build failed! Details on https://ci.trafficserver.apache.org/job/Github-Linux/33/
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219788326
After doing more reading here and on the net, I now agree with @pbchou in his original approach. Core should use `optind = 1` and all the plugins should be changed to use `optind = 0`. That's the safest and most cross platform approach.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219127222
Oops hit the wrong button.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218905456
Linux (CentOS7) build failed! Details on https://ci.trafficserver.apache.org/job/Github-Linux/40/
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-217458962
@jpeach Did you look at this ?
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219522840
IMHO resetting to 1 is the right fix. It should be safe to reset ``optind`` in ``plugin_load`` and remove all the plugin assumptions, since we guarantee that plugin loading is serialized. This should apply to remap instance loading as well.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219113313
I spent some time looking at this. I think the change is correct and I think it must be the plugin responsibility because we don't require plugins to use getopt. Here's the relevant language
> The variable optind is the index of the next element to be processed in argv. The system initializes this value to 1. The caller can reset it to 1 to restart scanning of the same argv, or when scanning a new argument vector.
I read this as indicating 1 is the correct value to use to reset `optind`.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-220376319
.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218906249
FreeBSD v10 build finished successfully. Details on https://ci.trafficserver.apache.org/job/Github-FreeBSD/19/
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219125801
On the Linux man page, at least, the optind should be set to 0 instead of 1 if you are mixing GNU extensions vs traditional between different invocations. If you set to 0, you force it to scan the opstring for GNU extensions, which ensures proper operation.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219549881
I think this depends on whether GNU extensions are being used or not since the man page does say that you "must" reinitialize with optind = 0 if you go back and forth between traditional and GNU extensions between scans. I am not certain what constitutes a GNU extension in the opstring, but I can report that two of the plugins included in this PR (background_fetch and regex_revalidate), which both use optind = 1 currently, do not play well together. background_fetch uses opstring "lc" while regex_revalidate uses "c:l:" so I suspect the colons are considered GNU extensions since that is the only difference besides the order of the letters. So sounds like glibc requires 0 and other libraries require 1 so this would need an #ifdef then. Note, there are actually 15 plugins using getopt (11 use optind=0 and 4 use optind=1).
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219565432
Heh, and so we are full circle. @jpeach Exactly, and that's what @pbchou said in an earlier post. To which I raised the concern about changing it to 0 (as the patch does) might not be cross platform.
If we want this Linux specific behavior of "0", doing what @PSUdaemon suggests is fine. What's somewhat amusing is that we have possibly broken plugins already that are using "0", in that they might not work on all platforms?
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by PSUdaemon <gi...@git.apache.org>.
Github user PSUdaemon commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219586100
Missing the `optarg = NULL` in the global case on purpose?
Also, I think we do want that `ifdef` around the `optind`. And I looked it up, we need to include `limits.h` and test for `__GLIBC__` on modern (post 1996) glibc's
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219194924
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218928614
Upgraded git on all the build boxes, so try again [approve ci].
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218525641
Can one of the admins verify this patch?
---
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 #571: TS-3245 : Changes optind = 1 to optind = 0 for all...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou commented on the issue:
https://github.com/apache/trafficserver/pull/571
Hi. I opened PR #696 under a new branch with a new patch based on the sample code provided by @jpeach. I had to re-base this branch on my local repository with master due to all of the intervening patches, but then I had difficulty pushing it into my GitHub fork. I didn't want to mess around with this branch in my fork in case it deleted these informative comments.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218782597
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218588544
[approve ci]
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-218903058
Another build needed [approve ci]
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219529521
But this patch is changing it to 0, which might be Linux only? But we do use this inconsistently, but then again, many plug-ins might not be tested on anything but Linux.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
Github user pbchou commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219565721
When there is an issue, typically the second plugin in the list will have corrupted values for its arguments as returned by getopt_long(). For example, it might return text from the first plugin's argument list.
One interesting thing is that all plugins are using getopt_long() and not getopt(). I think getopt_long() is itself a GNU extension according to the man page. So does this mean optind = 0 will always be safe (since implies GNU library) or have other non GNU libraries implemented getopt_long also?
Additional Notes:
+ There are 16 plugins making use of getopt_long(). There are 0 plugins using plain getopt(). The 15 mentioned in a prior comment was incorrect (-1 due to a local plugin, +2 due to two plugins not setting optind at all).
+ 10 plugins are using optind = 0.
+ 4 plugins are using optind = 1.
+ 2 plugins are using getopt_long but not setting optind at all. [epic, ssl_cert_loader]
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219190150
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219191874
Can one of the admins verify this patch? Only approve PRs which have been reviewed.
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by pbchou <gi...@git.apache.org>.
GitHub user pbchou reopened a pull request:
https://github.com/apache/trafficserver/pull/571
TS-3245 : Changes optind = 1 to optind = 0 for all plugins to ensure
that multiples plugins in the global plugin.config can co-exist
together. Note that all plugins were already optind = 0 except
for the four that were modified in the this commit (background_fetch,
regex_revalidate, remap_stats, and stale_while_revalidate).
Note: This patch does NOT change ATS core so that future plugins
must be sure to use optind = 0 also.
Understood preference is for ATS core change to cover all plugins, but this has been outstanding for more than one year and affects our usage of ATS. Also, I am not sure if an ATS core change can cover the plugins since according to the Linux man page setting optind=0 and calling getopt_long() causes an "internal" initialization process to run which looks at the optstring argument to getopt_long(). ATS core would not have pre-knowledge of each plugin's internals so this seems like this would be a road-block?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pbchou/trafficserver TS-3245
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/571.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 #571
----
commit a5141b2a90206d439841e1bd96aa5e8b15bcc9dd
Author: Peter Chou <pb...@labs.att.com>
Date: 2016-04-12T18:34:01Z
TS-3245 : Changes optind = 1 to optind = 0 for all plugins to ensure
that multiples plugins in the global plugin.config can co-exist
together. Note that all plugins were already optind = 0 except
for the four that were modified in the this commit (background_fetch,
regex_revalidate, remap_stats, and stale_while_revalidate).
Note: This patch does NOT change ATS core so that future plugins
must be sure to use optind = 0 also.
----
---
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: TS-3245 : Changes optind = 1 to optind...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/571#issuecomment-219586583
The missing ``optarg = NULL`` is just my fat fingers :)
---
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.
---