Re: pn_data_grow() issue
The JIRA project is at https://issues.apache.org/jira/browse/PROTON
Details of JIRA projects etc for each component are listed on the
website, e.g. http://qpid.apache.org/proton/ in this case.
Robbie
On 28 May 2015 at 20:46, Michael Ivanov iv...@isle.spb.ru wrote:
Sorry, how do I create a JIRA issue?
27.05.2015 16:48, Alan Conway пишет:
On Tue, 2015-05-26 at 23:25 +0300, Michael Ivanov wrote:
Greetings,
I have observed that pn_data_grow() function looses half of the available
data capacity.
The following happens: when data overflows, pn_data_grow is invoked. It
increases
data capacity 2 times and reallocates nodes array. Data capacity is
represented as
uint16_t type and so when capacity reaches 32768 items, the result of
multiplication by 2
becomes 0. This makes realloc return null and crashes the program.
To alleviate the problem with large messages I changed the function as
follows:
--- qpid-proton-0.9/proton-c/src/codec/codec.c 2015-03-31
12:07:22.0 +0300
+++ qpid-proton-0.9.fix/proton-c/src/codec/codec.c 2015-05-26
21:18:55.801632941 +0300
@@ -417,8 +417,21 @@ void pn_data_clear(pn_data_t *data)
int pn_data_grow(pn_data_t *data)
{
- data-capacity = 2*(data-capacity ? data-capacity : 2);
- data-nodes = (pni_node_t *) realloc(data-nodes, data-capacity *
sizeof(pni_node_t));
+ size_ts = data-capacity;
+
+ if (s 0x7fff)
+ s = 2 * (s? s : 2);
+ else if (s 0x - 1024)
+ s += 1024;
+ else if (s != 0x)
+ s = 0x;
+ else {
+ pn_logf(Data node %p overflow, data);
+ abort();
+ }
+
+ data-nodes = (pni_node_t *) realloc(data-nodes, s *
sizeof(pni_node_t));
+ data-capacity = s;
return 0;
This allows to use capacities in 0x8000 ... 0x range and is supposed to
report
data overflow.
Good catch, can you please create a JIRA with the patch attached so you
can get proper credit for it (for copyright reasons I can't take the
patch direct from email.) You can assign it to me, I'll apply it right
away.
I would suggest a couple of changes:
1. Define an explicit PNI_NID_MAX in data.h right beside typedef
pni_nid_t rather than using literal 0x etc. in case someone changes
the definition of pni_nid_t at some point.
2. Use a simple double or max rather than adding 1k after 0x7fff. We
need to fix the algorithm but I don't think we need to add new
complexity unless there's a separate justification for that.
3. If we fail to increase (i.e. we're already at PNI_NID_MAX) return
PN_OVERFLOW rather than aborting. There is only one function
(pn_data_new) that calls pn_data_grow, make it check the return value
and return NULL on error. That won't make exiting code worse (it will
still crash on overrun) but will give well written code that checks
error values the choice.
Thanks for this!
Alan.
--
\ / | |
(OvO) | Михаил Иванов|
(^^^) | Тел.:+7(911) 223-1300|
\^/ | E-mail: iv...@isle.spb.ru |
^ ^ | |
[jira] [Created] (PROTON-892) pn_data_t capacity does not grow above 32768 items
Michael created PROTON-892: -- Summary: pn_data_t capacity does not grow above 32768 items Key: PROTON-892 URL: https://issues.apache.org/jira/browse/PROTON-892 Project: Qpid Proton Issue Type: Improvement Components: proton-c Affects Versions: 0.9, 0.9.1 Reporter: Michael Priority: Minor pn_data_grow() function looses half of the available data capacity. The following happens: when data overflows, pn_data_grow is invoked. It increases data capacity 2 times and reallocates nodes array. Data capacity is represented as uint16_t type and so when capacity reaches 32768 items, the result of multiplication by 2 becomes 0. This makes realloc return null and crashes the program. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (PROTON-892) pn_data_t capacity does not grow above 32768 items
[ https://issues.apache.org/jira/browse/PROTON-892?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Michael updated PROTON-892: --- Attachment: 0003-data-capacity.patch Preliminary patch to alleviate the issue. When data capacity rreaches 32768 nodes, data grows is performed in 1024 items steps pn_data_t capacity does not grow above 32768 items -- Key: PROTON-892 URL: https://issues.apache.org/jira/browse/PROTON-892 Project: Qpid Proton Issue Type: Improvement Components: proton-c Affects Versions: 0.9, 0.9.1 Reporter: Michael Priority: Minor Attachments: 0003-data-capacity.patch pn_data_grow() function looses half of the available data capacity. The following happens: when data overflows, pn_data_grow is invoked. It increases data capacity 2 times and reallocates nodes array. Data capacity is represented as uint16_t type and so when capacity reaches 32768 items, the result of multiplication by 2 becomes 0. This makes realloc return null and crashes the program. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
Re: pn_data_grow() issue
Sorry, how do I create a JIRA issue?
27.05.2015 16:48, Alan Conway пишет:
On Tue, 2015-05-26 at 23:25 +0300, Michael Ivanov wrote:
Greetings,
I have observed that pn_data_grow() function looses half of the available
data capacity.
The following happens: when data overflows, pn_data_grow is invoked. It
increases
data capacity 2 times and reallocates nodes array. Data capacity is
represented as
uint16_t type and so when capacity reaches 32768 items, the result of
multiplication by 2
becomes 0. This makes realloc return null and crashes the program.
To alleviate the problem with large messages I changed the function as
follows:
--- qpid-proton-0.9/proton-c/src/codec/codec.c 2015-03-31
12:07:22.0 +0300
+++ qpid-proton-0.9.fix/proton-c/src/codec/codec.c 2015-05-26
21:18:55.801632941 +0300
@@ -417,8 +417,21 @@ void pn_data_clear(pn_data_t *data)
int pn_data_grow(pn_data_t *data)
{
- data-capacity = 2*(data-capacity ? data-capacity : 2);
- data-nodes = (pni_node_t *) realloc(data-nodes, data-capacity *
sizeof(pni_node_t));
+ size_ts = data-capacity;
+
+ if (s 0x7fff)
+ s = 2 * (s? s : 2);
+ else if (s 0x - 1024)
+ s += 1024;
+ else if (s != 0x)
+ s = 0x;
+ else {
+ pn_logf(Data node %p overflow, data);
+ abort();
+ }
+
+ data-nodes = (pni_node_t *) realloc(data-nodes, s * sizeof(pni_node_t));
+ data-capacity = s;
return 0;
This allows to use capacities in 0x8000 ... 0x range and is supposed to
report
data overflow.
Good catch, can you please create a JIRA with the patch attached so you
can get proper credit for it (for copyright reasons I can't take the
patch direct from email.) You can assign it to me, I'll apply it right
away.
I would suggest a couple of changes:
1. Define an explicit PNI_NID_MAX in data.h right beside typedef
pni_nid_t rather than using literal 0x etc. in case someone changes
the definition of pni_nid_t at some point.
2. Use a simple double or max rather than adding 1k after 0x7fff. We
need to fix the algorithm but I don't think we need to add new
complexity unless there's a separate justification for that.
3. If we fail to increase (i.e. we're already at PNI_NID_MAX) return
PN_OVERFLOW rather than aborting. There is only one function
(pn_data_new) that calls pn_data_grow, make it check the return value
and return NULL on error. That won't make exiting code worse (it will
still crash on overrun) but will give well written code that checks
error values the choice.
Thanks for this!
Alan.
--
\ / | |
(OvO) | Михаил Иванов|
(^^^) | Тел.:+7(911) 223-1300|
\^/ | E-mail: iv...@isle.spb.ru |
^ ^ | |
[jira] [Commented] (PROTON-885) Allow setup.py to bundle qpid-proton
[ https://issues.apache.org/jira/browse/PROTON-885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14563051#comment-14563051 ] ASF subversion and git services commented on PROTON-885: Commit a18a8484241ae7846fd1b5e832c66c3093110b99 in qpid-proton's branch refs/heads/0.9.x from [~kgiusti] [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=a18a848 ] PROTON-885: revert minimum version to 0.9 Allow setup.py to bundle qpid-proton Key: PROTON-885 URL: https://issues.apache.org/jira/browse/PROTON-885 Project: Qpid Proton Issue Type: Improvement Components: python-binding Reporter: Flavio Percoco Assignee: Ken Giusti Attachments: 0001-Allow-setup.py-for-bundling-proton.patch Allow setup.py for bundling proton As of now, it's not possible to install python-qpid-proton if libqpid-proton is not present in the system. To be more precises, it's possible to build python-qpid-proton using cmake, upload it and beg to the gods of OPs that the required (and correct) shared library will be present in the system. This patch adds to python-qpid-proton the ability to download, build and install qpid-proton if the required version is not present in the system. It does this by checking - using pkg-config - whether the required version is installed and if not, it goes to downloading the package from the official apache source and builds it using cmake. As nasty as it sounds, this process is not strange in the Python community. Very famous - and way more used - libraries like PyZMQ (from which this work took lots of inspiration) do this already in a fairly more complex way. This first step is quite simple, it checks, downloads and builds using the standard tools. It's enabled just for linux and it does not use fancy flags. Future enhancements could take care of improving the implementation and extending it to support other systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-885) Allow setup.py to bundle qpid-proton
[ https://issues.apache.org/jira/browse/PROTON-885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14563048#comment-14563048 ] ASF subversion and git services commented on PROTON-885: Commit 02e2eca694d5d430ea07cf363fe1ba98bb3a25af in qpid-proton's branch refs/heads/0.9.x from [~kgiusti] [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=02e2eca ] PROTON-885: add manifest, move license file into setuputils (cherry picked from commit c1f9ed41ac144f4cd451af4f3e0d0ba36dd1f203) Allow setup.py to bundle qpid-proton Key: PROTON-885 URL: https://issues.apache.org/jira/browse/PROTON-885 Project: Qpid Proton Issue Type: Improvement Components: python-binding Reporter: Flavio Percoco Assignee: Ken Giusti Attachments: 0001-Allow-setup.py-for-bundling-proton.patch Allow setup.py for bundling proton As of now, it's not possible to install python-qpid-proton if libqpid-proton is not present in the system. To be more precises, it's possible to build python-qpid-proton using cmake, upload it and beg to the gods of OPs that the required (and correct) shared library will be present in the system. This patch adds to python-qpid-proton the ability to download, build and install qpid-proton if the required version is not present in the system. It does this by checking - using pkg-config - whether the required version is installed and if not, it goes to downloading the package from the official apache source and builds it using cmake. As nasty as it sounds, this process is not strange in the Python community. Very famous - and way more used - libraries like PyZMQ (from which this work took lots of inspiration) do this already in a fairly more complex way. This first step is quite simple, it checks, downloads and builds using the standard tools. It's enabled just for linux and it does not use fancy flags. Future enhancements could take care of improving the implementation and extending it to support other systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-885) Allow setup.py to bundle qpid-proton
[ https://issues.apache.org/jira/browse/PROTON-885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14563050#comment-14563050 ] ASF subversion and git services commented on PROTON-885: Commit a117035e4a14ecf6d9a07f99966abd8efcaab6b7 in qpid-proton's branch refs/heads/0.9.x from [~flaper87] [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=a117035 ] PROTON-885: allow pip to set the install path for proton Allow setup.py to bundle qpid-proton Key: PROTON-885 URL: https://issues.apache.org/jira/browse/PROTON-885 Project: Qpid Proton Issue Type: Improvement Components: python-binding Reporter: Flavio Percoco Assignee: Ken Giusti Attachments: 0001-Allow-setup.py-for-bundling-proton.patch Allow setup.py for bundling proton As of now, it's not possible to install python-qpid-proton if libqpid-proton is not present in the system. To be more precises, it's possible to build python-qpid-proton using cmake, upload it and beg to the gods of OPs that the required (and correct) shared library will be present in the system. This patch adds to python-qpid-proton the ability to download, build and install qpid-proton if the required version is not present in the system. It does this by checking - using pkg-config - whether the required version is installed and if not, it goes to downloading the package from the official apache source and builds it using cmake. As nasty as it sounds, this process is not strange in the Python community. Very famous - and way more used - libraries like PyZMQ (from which this work took lots of inspiration) do this already in a fairly more complex way. This first step is quite simple, it checks, downloads and builds using the standard tools. It's enabled just for linux and it does not use fancy flags. Future enhancements could take care of improving the implementation and extending it to support other systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-885) Allow setup.py to bundle qpid-proton
[ https://issues.apache.org/jira/browse/PROTON-885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14563049#comment-14563049 ] ASF subversion and git services commented on PROTON-885: Commit ae6309f9ddacbb91785bed133c7d9885f18b5664 in qpid-proton's branch refs/heads/0.9.x from [~kgiusti] [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=ae6309f ] PROTON-885: add pthreads option to swig command (cherry picked from commit 0b7718c74a8fc905cc209b4859408feae5805029) Allow setup.py to bundle qpid-proton Key: PROTON-885 URL: https://issues.apache.org/jira/browse/PROTON-885 Project: Qpid Proton Issue Type: Improvement Components: python-binding Reporter: Flavio Percoco Assignee: Ken Giusti Attachments: 0001-Allow-setup.py-for-bundling-proton.patch Allow setup.py for bundling proton As of now, it's not possible to install python-qpid-proton if libqpid-proton is not present in the system. To be more precises, it's possible to build python-qpid-proton using cmake, upload it and beg to the gods of OPs that the required (and correct) shared library will be present in the system. This patch adds to python-qpid-proton the ability to download, build and install qpid-proton if the required version is not present in the system. It does this by checking - using pkg-config - whether the required version is installed and if not, it goes to downloading the package from the official apache source and builds it using cmake. As nasty as it sounds, this process is not strange in the Python community. Very famous - and way more used - libraries like PyZMQ (from which this work took lots of inspiration) do this already in a fairly more complex way. This first step is quite simple, it checks, downloads and builds using the standard tools. It's enabled just for linux and it does not use fancy flags. Future enhancements could take care of improving the implementation and extending it to support other systems. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
