You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by mg...@apache.org on 2015/07/02 23:38:33 UTC
qpid-proton git commit: PROTON-925: use scanner right for
remote_channel_max and remote_max_frame
Repository: qpid-proton
Updated Branches:
refs/heads/master 7e3190306 -> fc38e86a6
PROTON-925: use scanner right for remote_channel_max and remote_max_frame
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/fc38e86a
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/fc38e86a
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/fc38e86a
Branch: refs/heads/master
Commit: fc38e86a6f5a1b265552708e674d3c8040c1985b
Parents: 7e31903
Author: mgoulish <mi...@redhat.com>
Authored: Thu Jul 2 16:43:45 2015 -0400
Committer: mgoulish <mi...@redhat.com>
Committed: Thu Jul 2 16:43:45 2015 -0400
----------------------------------------------------------------------
proton-c/src/transport/transport.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/fc38e86a/proton-c/src/transport/transport.c
----------------------------------------------------------------------
diff --git a/proton-c/src/transport/transport.c b/proton-c/src/transport/transport.c
index a4b07c3..36eeb00 100644
--- a/proton-c/src/transport/transport.c
+++ b/proton-c/src/transport/transport.c
@@ -396,7 +396,7 @@ static void pn_transport_initialize(void *object)
transport->remote_container = NULL;
transport->remote_hostname = NULL;
transport->local_max_frame = PN_DEFAULT_MAX_FRAME_SIZE;
- transport->remote_max_frame = 0;
+ transport->remote_max_frame = UINT32_MAX;
/*
* We set the local limit on channels to 2^15, because
@@ -1101,20 +1101,37 @@ static char *pn_bytes_strdup(pn_bytes_t str)
int pn_do_open(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
{
pn_connection_t *conn = transport->connection;
- bool container_q, hostname_q;
+ bool container_q, hostname_q, remote_channel_max_q, remote_max_frame_q;
+ uint16_t remote_channel_max;
+ uint32_t remote_max_frame;
pn_bytes_t remote_container, remote_hostname;
pn_data_clear(transport->remote_offered_capabilities);
pn_data_clear(transport->remote_desired_capabilities);
pn_data_clear(transport->remote_properties);
- int err = pn_data_scan(args, "D.[?S?SIHI..CCC]", &container_q,
- &remote_container, &hostname_q, &remote_hostname,
- &transport->remote_max_frame,
- &transport->remote_channel_max,
+ int err = pn_data_scan(args, "D.[?S?S?I?HI..CCC]",
+ &container_q, &remote_container,
+ &hostname_q, &remote_hostname,
+ &remote_max_frame_q, &remote_max_frame,
+ &remote_channel_max_q, &remote_channel_max,
&transport->remote_idle_timeout,
transport->remote_offered_capabilities,
transport->remote_desired_capabilities,
transport->remote_properties);
if (err) return err;
+ /*
+ * The default value is already stored in the variable.
+ * But the scanner zeroes out values if it does not
+ * find them in the args, so don't give the variable
+ * directly to the scanner.
+ */
+ if (remote_channel_max_q) {
+ transport->remote_channel_max = remote_channel_max;
+ }
+
+ if (remote_max_frame_q) {
+ transport->remote_max_frame = remote_max_frame;
+ }
+
if (transport->remote_max_frame > 0) {
if (transport->remote_max_frame < AMQP_MIN_MAX_FRAME_SIZE) {
pn_transport_logf(transport, "Peer advertised bad max-frame (%u), forcing to %u",
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
Re: qpid-proton git commit: PROTON-925: use scanner right for remote_channel_max
and remote_max_frame
Posted by Gordon Sim <gs...@redhat.com>.
On 07/02/2015 10:38 PM, mgoulish@apache.org wrote:
> Repository: qpid-proton
> Updated Branches:
> refs/heads/master 7e3190306 -> fc38e86a6
>
>
> PROTON-925: use scanner right for remote_channel_max and remote_max_frame
Was the change for remote_max_frame necessary? Unlike channel-max, 0
would be an invalid value for this and there were already checks in
place regarding the value specified.
Have you done any testing around the handling of specified and
unspecified max frame sizes to ensure it works as expected? The code
previously expected the value of remote_max_frame to be 0 if
unspecified; have you verified that you haven't broken anything by
changing this?
(I don't actually think you do break anything, but my concern is whether
the implications of changing the max frame value were fully considered,
or if this was just tacked on to the change for channel_max which is a
quite different issue.)
> Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
> Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/fc38e86a
> Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/fc38e86a
> Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/fc38e86a
>
> Branch: refs/heads/master
> Commit: fc38e86a6f5a1b265552708e674d3c8040c1985b
> Parents: 7e31903
> Author: mgoulish <mi...@redhat.com>
> Authored: Thu Jul 2 16:43:45 2015 -0400
> Committer: mgoulish <mi...@redhat.com>
> Committed: Thu Jul 2 16:43:45 2015 -0400
>
> ----------------------------------------------------------------------
> proton-c/src/transport/transport.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/fc38e86a/proton-c/src/transport/transport.c
> ----------------------------------------------------------------------
> diff --git a/proton-c/src/transport/transport.c b/proton-c/src/transport/transport.c
> index a4b07c3..36eeb00 100644
> --- a/proton-c/src/transport/transport.c
> +++ b/proton-c/src/transport/transport.c
> @@ -396,7 +396,7 @@ static void pn_transport_initialize(void *object)
> transport->remote_container = NULL;
> transport->remote_hostname = NULL;
> transport->local_max_frame = PN_DEFAULT_MAX_FRAME_SIZE;
> - transport->remote_max_frame = 0;
> + transport->remote_max_frame = UINT32_MAX;
>
> /*
> * We set the local limit on channels to 2^15, because
> @@ -1101,20 +1101,37 @@ static char *pn_bytes_strdup(pn_bytes_t str)
> int pn_do_open(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
> {
> pn_connection_t *conn = transport->connection;
> - bool container_q, hostname_q;
> + bool container_q, hostname_q, remote_channel_max_q, remote_max_frame_q;
> + uint16_t remote_channel_max;
> + uint32_t remote_max_frame;
> pn_bytes_t remote_container, remote_hostname;
> pn_data_clear(transport->remote_offered_capabilities);
> pn_data_clear(transport->remote_desired_capabilities);
> pn_data_clear(transport->remote_properties);
> - int err = pn_data_scan(args, "D.[?S?SIHI..CCC]", &container_q,
> - &remote_container, &hostname_q, &remote_hostname,
> - &transport->remote_max_frame,
> - &transport->remote_channel_max,
> + int err = pn_data_scan(args, "D.[?S?S?I?HI..CCC]",
> + &container_q, &remote_container,
> + &hostname_q, &remote_hostname,
> + &remote_max_frame_q, &remote_max_frame,
> + &remote_channel_max_q, &remote_channel_max,
> &transport->remote_idle_timeout,
> transport->remote_offered_capabilities,
> transport->remote_desired_capabilities,
> transport->remote_properties);
> if (err) return err;
> + /*
> + * The default value is already stored in the variable.
> + * But the scanner zeroes out values if it does not
> + * find them in the args, so don't give the variable
> + * directly to the scanner.
> + */
> + if (remote_channel_max_q) {
> + transport->remote_channel_max = remote_channel_max;
> + }
> +
> + if (remote_max_frame_q) {
> + transport->remote_max_frame = remote_max_frame;
> + }
> +
> if (transport->remote_max_frame > 0) {
> if (transport->remote_max_frame < AMQP_MIN_MAX_FRAME_SIZE) {
> pn_transport_logf(transport, "Peer advertised bad max-frame (%u), forcing to %u",
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
> For additional commands, e-mail: commits-help@qpid.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org