You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@httpd.apache.org by GitBox <gi...@apache.org> on 2020/07/10 11:37:09 UTC
[GitHub] [httpd] uhliarik opened a new pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
uhliarik opened a new pull request #133:
URL: https://github.com/apache/httpd/pull/133
…params
now (previously had only 3). Pass dummy integer variable as 4th param, if there
is function definition with 4 params.
I didn't have opportunity to run test suite on it, so feel free to review it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] wrowe commented on pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
Posted by GitBox <gi...@apache.org>.
wrowe commented on pull request #133:
URL: https://github.com/apache/httpd/pull/133#issuecomment-665095226
Something important to note, the logic in mod_lua confused me... we are ignoring the 2nd arg altogether. I'm not clear why this choice was made since the code and the lua docs don't explain. Given that discrepancy, I'm not entirely sure this works in practice.
Second important thing to note, we wouldn't know, because the test suite doesn't exercise lua_resume.
The patch certainly builds, so the PR is certainly interesting, thanks for submitting!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] asfgit closed pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #133:
URL: https://github.com/apache/httpd/pull/133
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] notroj commented on pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
Posted by GitBox <gi...@apache.org>.
notroj commented on pull request #133:
URL: https://github.com/apache/httpd/pull/133#issuecomment-665104105
Yeah, I also couldn't really understand what the new argument is doing or whether it is really needed here. :(
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] notroj commented on pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
Posted by GitBox <gi...@apache.org>.
notroj commented on pull request #133:
URL: https://github.com/apache/httpd/pull/133#issuecomment-670449709
So the lua filtering API uses a "yield" from the filter code to pass back bucket data. It seems to assume currently that after a yield exactly one item is present on the stack, e.g. here https://github.com/apache/httpd/blob/trunk/modules/lua/mod_lua.c#L495
I guess there is some way to determine the stack size with <5.4 but haven't checked. In #134 I've taken @uhliarik and added checking that the stack size is exactly one using the 5.4 api. I think this makes sense but not very familiar with the code.
Also added a test for the output filters in http://svn.apache.org/viewvc?view=revision&revision=1880665 to see if this breaks anything for <5.4 as tested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rainerjung commented on pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
Posted by GitBox <gi...@apache.org>.
rainerjung commented on pull request #133:
URL: https://github.com/apache/httpd/pull/133#issuecomment-665627329
From http://www.lua.org/manual/5.4/manual.html#8.3:
The function lua_resume has an extra parameter. This out parameter returns the number of values on the top of the stack that were yielded or returned by the coroutine. (In previous versions, those values were the entire stack.)
I don't know, what this means for mod_lua, just wanted to provide a link to the param explanation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] notroj edited a comment on pull request #133: In Lua 5.4, there is new lua_resume function definition, which has 4 …
Posted by GitBox <gi...@apache.org>.
notroj edited a comment on pull request #133:
URL: https://github.com/apache/httpd/pull/133#issuecomment-670449709
So the lua filtering API uses a "yield" from the filter code to pass back bucket data. It seems to assume currently that after a yield exactly one item is present on the stack, e.g. here https://github.com/apache/httpd/blob/trunk/modules/lua/mod_lua.c#L495
I guess there is some way to determine the stack size with <5.4 but haven't checked. In #134 I've taken the patch from @uhliarik and added checking that the stack size is exactly one using the 5.4 api. I think this makes sense but not very familiar with the code.
Also added a test for the output filters in http://svn.apache.org/viewvc?view=revision&revision=1880665 to see if this breaks anything for <5.4 as tested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org