You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by "Darryl L. Pierce" <dp...@redhat.com> on 2012/08/23 21:27:24 UTC
pn_message_save_data interesting problem...
In working on the stable Ruby APIs on top of Swig I've hit an
interesting problem with this API.
The stable API for this is Qpid::Proton::get_message_data(msg, size)
and returns an array that is the size of the string and the string
itself, respectively. The swig wrapper currently is:
%rename (pn_message_save) wrap_pn_message_save;
%inline %{
int wrap_pn_message_save(pn_message_t *msg, char *OUTPUT, size_t *OUTPUT_SIZE) {
ssize_t sz = pn_message_save(msg, OUTPUT, OUTPUT_SIZE);
if (sz < 0) *OUTPUT_SIZE = 0;
return *OUTPUT_SIZE;
}
%}
%ignore pn_message_save;
One test I've written is to generate random strings of various lengths,
throwing them at the message and then getting them back to verify they
were saved properly. And what I'm seeing happening is that, at certain
lengths (2 specifically), the APIs fail.
Initially, when the length hits either 129 or 257 bytes, the string
returned does not match the string submitted. If it gets past 129 then
it ALWAYS fails at 257 bytes. And if I tell it to ignore anything before
257 then it always fails at 513.
Not sure on first glance what's causing the problem.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
Re: pn_message_save_data interesting problem...
Posted by "Darryl L. Pierce" <dp...@redhat.com>.
On Mon, Aug 27, 2012 at 05:17:26PM -0400, William Henry wrote:
> > On Fri, Aug 24, 2012 at 11:12:26AM -0400, Darryl L. Pierce wrote:
> > > [TEST AT 64 CHARACTERS]
> > > Testing data at 64 characters.
> > > Trying with a bufsize of 1024.
> > > rc=0
> > > bufsize=64
> > > *ALLOC_OUTPUT is 65 bytes.
> > > *ALLOC_OUTPUT ==
> > > "vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcgP"
> > > Exiting with a return code of 64
> > > Comparing:
> > > vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> > > vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> > > Do they match? Yes
> > >
> > > [TEST AT 65 CHARACTERS]
> > > Testing data at 65 characters.
> > > Trying with a bufsize of 1024.
> > > rc=0
> > > bufsize=65
> > > *ALLOC_OUTPUT is 4 bytes.
> > > *ALLOC_OUTPUT == "�j� "
> > > Exiting with a return code of 65
> > > Comparing:
> > > obinopuaxubdehrwmkqqxhitzcfroiqnzdoxjqcqvkgpwsdprryegwxluiheklaul
> > > �j�
> > >
> > > pkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> > > Do they match? No
> > >
> > > As you can, at 64 bytes (and all lengths prior to it) the strings
> > > internal always match. But, at 65 bytes the result is those weird
> > > few
> > > characters and nothing even remotely like the string passed in
> > > previously.
> >
> > More investigation and debuggin on this:
> >
> > When the string fails (you can see it in the above case) the failed
> > string consists some garbage and then the previous message's string
> > starting at n bytes into that string, where n = 8 if the length is
> > 65, n
> > = 16 if length is 129 or 257.
> >
> > Still digging into this but haven't found where the problem is. Still
> > wrapping my head around the encoding and decoding pieces.
>
> As I'm reading this "I Still Haven't Found What I'm Looking For" from U2's Joshua Tree was playing in the background.
>
> Where is the offending code? What is ALLOC_OUTPUT for? Why is it 4?
ALLOC_OUTPUT is in the ruby.i file, in the wrap_pn_message_save()
wrapper. As best I can tell, it's 4 bytes because *ALLOC_OUTPUT+5 is a
null byte at this point, even though the input was 65 bytes.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
Re: pn_message_save_data interesting problem...
Posted by William Henry <wh...@redhat.com>.
----- Original Message -----
> On Fri, Aug 24, 2012 at 11:12:26AM -0400, Darryl L. Pierce wrote:
> > On Fri, Aug 24, 2012 at 10:27:29AM -0400, Chuck Rolke wrote:
> > > This sounds like a gratuitous trailing null.
> > > Added nulls could be happening on all buffer sizes but don't
> > > cause an issue until the power-of-2-size payload puts the null
> > > into territory that gets checked for buffer overruns.
> >
> > I think you're onto something there. Putting some debugging code
> > into
> > the wrapper, I'm seeing that at those points the data returns by
> > pn_message_save is corrupted; i.e., on those specific lengths the
> > data returns doesn't even remotely match. For example:
> >
> > [TEST AT 64 CHARACTERS]
> > Testing data at 64 characters.
> > Trying with a bufsize of 1024.
> > rc=0
> > bufsize=64
> > *ALLOC_OUTPUT is 65 bytes.
> > *ALLOC_OUTPUT ==
> > "vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcgP"
> > Exiting with a return code of 64
> > Comparing:
> > vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> > vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> > Do they match? Yes
> >
> > [TEST AT 65 CHARACTERS]
> > Testing data at 65 characters.
> > Trying with a bufsize of 1024.
> > rc=0
> > bufsize=65
> > *ALLOC_OUTPUT is 4 bytes.
> > *ALLOC_OUTPUT == "�j� "
> > Exiting with a return code of 65
> > Comparing:
> > obinopuaxubdehrwmkqqxhitzcfroiqnzdoxjqcqvkgpwsdprryegwxluiheklaul
> > �j�
> >
> > pkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> > Do they match? No
> >
> > As you can, at 64 bytes (and all lengths prior to it) the strings
> > internal always match. But, at 65 bytes the result is those weird
> > few
> > characters and nothing even remotely like the string passed in
> > previously.
>
> More investigation and debuggin on this:
>
> When the string fails (you can see it in the above case) the failed
> string consists some garbage and then the previous message's string
> starting at n bytes into that string, where n = 8 if the length is
> 65, n
> = 16 if length is 129 or 257.
>
> Still digging into this but haven't found where the problem is. Still
> wrapping my head around the encoding and decoding pieces.
As I'm reading this "I Still Haven't Found What I'm Looking For" from U2's Joshua Tree was playing in the background.
Where is the offending code? What is ALLOC_OUTPUT for? Why is it 4?
William
>
> --
> Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
>
>
Re: pn_message_save_data interesting problem...
Posted by "Darryl L. Pierce" <dp...@redhat.com>.
On Fri, Aug 24, 2012 at 11:12:26AM -0400, Darryl L. Pierce wrote:
> On Fri, Aug 24, 2012 at 10:27:29AM -0400, Chuck Rolke wrote:
> > This sounds like a gratuitous trailing null.
> > Added nulls could be happening on all buffer sizes but don't cause an issue until the power-of-2-size payload puts the null into territory that gets checked for buffer overruns.
>
> I think you're onto something there. Putting some debugging code into
> the wrapper, I'm seeing that at those points the data returns by
> pn_message_save is corrupted; i.e., on those specific lengths the
> data returns doesn't even remotely match. For example:
>
> [TEST AT 64 CHARACTERS]
> Testing data at 64 characters.
> Trying with a bufsize of 1024.
> rc=0
> bufsize=64
> *ALLOC_OUTPUT is 65 bytes.
> *ALLOC_OUTPUT ==
> "vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcgP"
> Exiting with a return code of 64
> Comparing:
> vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> Do they match? Yes
>
> [TEST AT 65 CHARACTERS]
> Testing data at 65 characters.
> Trying with a bufsize of 1024.
> rc=0
> bufsize=65
> *ALLOC_OUTPUT is 4 bytes.
> *ALLOC_OUTPUT == "�j� "
> Exiting with a return code of 65
> Comparing:
> obinopuaxubdehrwmkqqxhitzcfroiqnzdoxjqcqvkgpwsdprryegwxluiheklaul
> �j�
>
> pkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> Do they match? No
>
> As you can, at 64 bytes (and all lengths prior to it) the strings
> internal always match. But, at 65 bytes the result is those weird few
> characters and nothing even remotely like the string passed in
> previously.
More investigation and debuggin on this:
When the string fails (you can see it in the above case) the failed
string consists some garbage and then the previous message's string
starting at n bytes into that string, where n = 8 if the length is 65, n
= 16 if length is 129 or 257.
Still digging into this but haven't found where the problem is. Still
wrapping my head around the encoding and decoding pieces.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
Re: pn_message_save_data interesting problem...
Posted by William Henry <wh...@redhat.com>.
Too funny. Ok so "word" in my reply was the wrong word but I did say you have corruption going on at these boundaries and an assert will help you diagnose. But don't mind me.
This is a classic and pretty common issue.
William
----- Original Message -----
> On Fri, Aug 24, 2012 at 10:27:29AM -0400, Chuck Rolke wrote:
> > This sounds like a gratuitous trailing null.
> > Added nulls could be happening on all buffer sizes but don't cause
> > an issue until the power-of-2-size payload puts the null into
> > territory that gets checked for buffer overruns.
>
> I think you're onto something there. Putting some debugging code into
> the wrapper, I'm seeing that at those points the data returns by
> pn_message_save is corrupted; i.e., on those specific lengths the
> data returns doesn't even remotely match. For example:
>
> [TEST AT 64 CHARACTERS]
> Testing data at 64 characters.
> Trying with a bufsize of 1024.
> rc=0
> bufsize=64
> *ALLOC_OUTPUT is 65 bytes.
> *ALLOC_OUTPUT ==
> "vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcgP"
> Exiting with a return code of 64
> Comparing:
> vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> Do they match? Yes
>
> [TEST AT 65 CHARACTERS]
> Testing data at 65 characters.
> Trying with a bufsize of 1024.
> rc=0
> bufsize=65
> *ALLOC_OUTPUT is 4 bytes.
> *ALLOC_OUTPUT == "�j� "
> Exiting with a return code of 65
> Comparing:
> obinopuaxubdehrwmkqqxhitzcfroiqnzdoxjqcqvkgpwsdprryegwxluiheklaul
> �j�
>
> pkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
> Do they match? No
>
> As you can, at 64 bytes (and all lengths prior to it) the strings
> internal always match. But, at 65 bytes the result is those weird few
> characters and nothing even remotely like the string passed in
> previously.
>
> --
> Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
>
>
Re: pn_message_save_data interesting problem...
Posted by "Darryl L. Pierce" <dp...@redhat.com>.
On Fri, Aug 24, 2012 at 10:27:29AM -0400, Chuck Rolke wrote:
> This sounds like a gratuitous trailing null.
> Added nulls could be happening on all buffer sizes but don't cause an issue until the power-of-2-size payload puts the null into territory that gets checked for buffer overruns.
I think you're onto something there. Putting some debugging code into
the wrapper, I'm seeing that at those points the data returns by
pn_message_save is corrupted; i.e., on those specific lengths the
data returns doesn't even remotely match. For example:
[TEST AT 64 CHARACTERS]
Testing data at 64 characters.
Trying with a bufsize of 1024.
rc=0
bufsize=64
*ALLOC_OUTPUT is 65 bytes.
*ALLOC_OUTPUT ==
"vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcgP"
Exiting with a return code of 64
Comparing:
vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
vdzxprjepkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
Do they match? Yes
[TEST AT 65 CHARACTERS]
Testing data at 65 characters.
Trying with a bufsize of 1024.
rc=0
bufsize=65
*ALLOC_OUTPUT is 4 bytes.
*ALLOC_OUTPUT == "�j� "
Exiting with a return code of 65
Comparing:
obinopuaxubdehrwmkqqxhitzcfroiqnzdoxjqcqvkgpwsdprryegwxluiheklaul
�j�
pkvgcfzdtzhdrkuambduahhjmhdczkcihuauedxzelgcjqlxfnhqpmcg
Do they match? No
As you can, at 64 bytes (and all lengths prior to it) the strings
internal always match. But, at 65 bytes the result is those weird few
characters and nothing even remotely like the string passed in
previously.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
Re: pn_message_save_data interesting problem...
Posted by Chuck Rolke <cr...@redhat.com>.
This sounds like a gratuitous trailing null.
Added nulls could be happening on all buffer sizes but don't cause an issue until the power-of-2-size payload puts the null into territory that gets checked for buffer overruns.
----- Original Message -----
> From: "Darryl L. Pierce" <dp...@redhat.com>
> To: proton@qpid.apache.org
> Sent: Friday, August 24, 2012 9:38:14 AM
> Subject: Re: pn_message_save_data interesting problem...
>
> On Thu, Aug 23, 2012 at 05:39:02PM -0400, Rafael Schloming wrote:
> > On Thu, Aug 23, 2012 at 5:06 PM, Darryl L. Pierce
> > <dp...@redhat.com>wrote:
> >
> > > On Thu, Aug 23, 2012 at 04:02:09PM -0400, Rafael Schloming wrote:
> > > > I'm a little confused by what you're trying to do with the
> > > > wrapper. This
> > > is
> > > > what I have for the wrapper in my ruby.i:
> > > >
> > > > int pn_message_save(pn_message_t *msg, char *OUTPUT, size_t
> > > *OUTPUT_SIZE);
> > > > %ignore pn_message_save;
> > >
> > > With the above the error happens at 65 characters. I took out my
> > > wrapper
> > > and ran the test again and it crashes at 65 characters every
> > > time.
> > >
> >
> > Can you post the test snippet so I can try?
>
> If you track my "Ruby-language-APIs" branch on github[1], and run the
> following in the ruby directory with
>
> ruby -I $RUBYLIB -I lib test
>
> you should see the output.
>
> ---8<[snip]---
> #!/usr/bin/env ruby
>
> require 'qpid_proton'
>
> msg = Qpid::Proton.create_message
> Qpid::Proton.set_message_format(msg, Qpid::Proton::Format::TEXT)
> length = 1
>
> puts "The message format is #{Qpid::Proton.get_message_format(msg)}."
>
> loop do
> data = (0...length).map{ ('a'..'z').to_a[rand(26)] }.join
>
> puts "Testing data at #{length} characters."
>
> Qpid::Proton.set_message_data(msg, data)
>
> returned = Qpid::Proton.get_message_data(msg)[1]
>
> puts "Comparing:\n#{data}\n#{returned}\n"
> puts "Do they match? #{(data == returned) ? 'Yes' : 'No'}"
>
> break unless (data == returned)
>
> Qpid::Proton.clear_message(msg)
>
> # length = 0 if length == 128
>
> length += 1
>
> end
>
> puts "We stopped at #{length} characters."
> ---8<[snip]---
>
> > > > I believe this is all that is necessary since the standard
> > > > typemaps
> > > should
> > > > pick up on OUTPUT and OUTPUT_SIZE and just do the right thing.
> > >
> > > What I would like to see in the dynamic languages for this is to
> > > not
> > > even have to specify the size for the output buffer; i.e., just
> > > call:
> > >
> > > Qpid::Proton.get_message_data(msg)
> > >
> > > and get the data back as one lump.
> > >
> >
> > Ah, in that case I think you want to use ALLOC_OUTPUT and
> > ALLOC_SIZE. Your
> > wrapper will need to be a little bit smarter though. There is no
> > way to
> > know in advance how much space it will take to encode the message
> > in a
> > given format, so the wrapper will have to pick an initial size and
> > then
> > keep on doubling until it fits.
> >
> > Here is an example of the ALLOC stuff. Obviously the wrapper would
> > need to
> > be different.
> >
> > %rename(pn_delivery_tag) wrap_pn_delivery_tag;
> > %inline %{
> > void wrap_pn_delivery_tag(pn_delivery_t *delivery, char
> > **ALLOC_OUTPUT,
> > size_t *ALLOC_SIZE) {
> > pn_delivery_tag_t tag = pn_delivery_tag(delivery);
> > *ALLOC_OUTPUT = malloc(tag.size);
> > *ALLOC_SIZE = tag.size;
> > memcpy(*ALLOC_OUTPUT, tag.bytes, tag.size);
> > }
> > %}
> > %ignore pn_delivery_tag;
> >
> > The swig chapter on C string handle is a good reference for this
> > stuff.
> > There might be some other macros there that are useful:
> >
> > http://www.swig.org/Doc2.0/SWIGDocumentation.html#Library_nn8
>
> Okay, I'll take a different approach to this one.
>
> > > Agreed. Also, we should make sure to remove having the individual
> > > languages include them relatively (i.e., %include "../cproton.i")
> > > and
> > > instead place the shared files in the include directory.
> > >
> >
> > +1
>
> I'll send out a patch for this today.
>
> --
> Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
>
>
Re: pn_message_save_data interesting problem...
Posted by "Darryl L. Pierce" <dp...@redhat.com>.
On Thu, Aug 23, 2012 at 05:39:02PM -0400, Rafael Schloming wrote:
> On Thu, Aug 23, 2012 at 5:06 PM, Darryl L. Pierce <dp...@redhat.com>wrote:
>
> > On Thu, Aug 23, 2012 at 04:02:09PM -0400, Rafael Schloming wrote:
> > > I'm a little confused by what you're trying to do with the wrapper. This
> > is
> > > what I have for the wrapper in my ruby.i:
> > >
> > > int pn_message_save(pn_message_t *msg, char *OUTPUT, size_t
> > *OUTPUT_SIZE);
> > > %ignore pn_message_save;
> >
> > With the above the error happens at 65 characters. I took out my wrapper
> > and ran the test again and it crashes at 65 characters every time.
> >
>
> Can you post the test snippet so I can try?
If you track my "Ruby-language-APIs" branch on github[1], and run the
following in the ruby directory with
ruby -I $RUBYLIB -I lib test
you should see the output.
---8<[snip]---
#!/usr/bin/env ruby
require 'qpid_proton'
msg = Qpid::Proton.create_message
Qpid::Proton.set_message_format(msg, Qpid::Proton::Format::TEXT)
length = 1
puts "The message format is #{Qpid::Proton.get_message_format(msg)}."
loop do
data = (0...length).map{ ('a'..'z').to_a[rand(26)] }.join
puts "Testing data at #{length} characters."
Qpid::Proton.set_message_data(msg, data)
returned = Qpid::Proton.get_message_data(msg)[1]
puts "Comparing:\n#{data}\n#{returned}\n"
puts "Do they match? #{(data == returned) ? 'Yes' : 'No'}"
break unless (data == returned)
Qpid::Proton.clear_message(msg)
# length = 0 if length == 128
length += 1
end
puts "We stopped at #{length} characters."
---8<[snip]---
> > > I believe this is all that is necessary since the standard typemaps
> > should
> > > pick up on OUTPUT and OUTPUT_SIZE and just do the right thing.
> >
> > What I would like to see in the dynamic languages for this is to not
> > even have to specify the size for the output buffer; i.e., just call:
> >
> > Qpid::Proton.get_message_data(msg)
> >
> > and get the data back as one lump.
> >
>
> Ah, in that case I think you want to use ALLOC_OUTPUT and ALLOC_SIZE. Your
> wrapper will need to be a little bit smarter though. There is no way to
> know in advance how much space it will take to encode the message in a
> given format, so the wrapper will have to pick an initial size and then
> keep on doubling until it fits.
>
> Here is an example of the ALLOC stuff. Obviously the wrapper would need to
> be different.
>
> %rename(pn_delivery_tag) wrap_pn_delivery_tag;
> %inline %{
> void wrap_pn_delivery_tag(pn_delivery_t *delivery, char **ALLOC_OUTPUT,
> size_t *ALLOC_SIZE) {
> pn_delivery_tag_t tag = pn_delivery_tag(delivery);
> *ALLOC_OUTPUT = malloc(tag.size);
> *ALLOC_SIZE = tag.size;
> memcpy(*ALLOC_OUTPUT, tag.bytes, tag.size);
> }
> %}
> %ignore pn_delivery_tag;
>
> The swig chapter on C string handle is a good reference for this stuff.
> There might be some other macros there that are useful:
>
> http://www.swig.org/Doc2.0/SWIGDocumentation.html#Library_nn8
Okay, I'll take a different approach to this one.
> > Agreed. Also, we should make sure to remove having the individual
> > languages include them relatively (i.e., %include "../cproton.i") and
> > instead place the shared files in the include directory.
> >
>
> +1
I'll send out a patch for this today.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
Re: pn_message_save_data interesting problem...
Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Thu, Aug 23, 2012 at 5:06 PM, Darryl L. Pierce <dp...@redhat.com>wrote:
> On Thu, Aug 23, 2012 at 04:02:09PM -0400, Rafael Schloming wrote:
> > I'm a little confused by what you're trying to do with the wrapper. This
> is
> > what I have for the wrapper in my ruby.i:
> >
> > int pn_message_save(pn_message_t *msg, char *OUTPUT, size_t
> *OUTPUT_SIZE);
> > %ignore pn_message_save;
>
> With the above the error happens at 65 characters. I took out my wrapper
> and ran the test again and it crashes at 65 characters every time.
>
Can you post the test snippet so I can try?
>
> > I believe this is all that is necessary since the standard typemaps
> should
> > pick up on OUTPUT and OUTPUT_SIZE and just do the right thing.
>
> What I would like to see in the dynamic languages for this is to not
> even have to specify the size for the output buffer; i.e., just call:
>
> Qpid::Proton.get_message_data(msg)
>
> and get the data back as one lump.
>
Ah, in that case I think you want to use ALLOC_OUTPUT and ALLOC_SIZE. Your
wrapper will need to be a little bit smarter though. There is no way to
know in advance how much space it will take to encode the message in a
given format, so the wrapper will have to pick an initial size and then
keep on doubling until it fits.
Here is an example of the ALLOC stuff. Obviously the wrapper would need to
be different.
%rename(pn_delivery_tag) wrap_pn_delivery_tag;
%inline %{
void wrap_pn_delivery_tag(pn_delivery_t *delivery, char **ALLOC_OUTPUT,
size_t *ALLOC_SIZE) {
pn_delivery_tag_t tag = pn_delivery_tag(delivery);
*ALLOC_OUTPUT = malloc(tag.size);
*ALLOC_SIZE = tag.size;
memcpy(*ALLOC_OUTPUT, tag.bytes, tag.size);
}
%}
%ignore pn_delivery_tag;
The swig chapter on C string handle is a good reference for this stuff.
There might be some other macros there that are useful:
http://www.swig.org/Doc2.0/SWIGDocumentation.html#Library_nn8
> > Regardless, I think the cause for the issue is that your wrapper function
> > is assuming pn_message_save returns a signed size, but actually it
> returns
> > an int that is 0 on success or an error code on failure:
> >
> > int pn_message_save(pn_message_t *message, char *data, size_t *size);
> >
> > On success the size of the output data is stored into *size. (I'll add
> some
> > API-doc for this.)
> >
> > As an aside in looking at this I noticed that there is some duplication
> > between the php, ruby, and python files that could be consolidated into
> the
> > cproton.i. I think this particular declaration is part of that
> duplication
> > unless there is some reason I'm missing that it has to be different for
> > ruby.
>
> Agreed. Also, we should make sure to remove having the individual
> languages include them relatively (i.e., %include "../cproton.i") and
> instead place the shared files in the include directory.
>
+1
--Rafael
Re: pn_message_save_data interesting problem...
Posted by "Darryl L. Pierce" <dp...@redhat.com>.
On Thu, Aug 23, 2012 at 04:02:09PM -0400, Rafael Schloming wrote:
> I'm a little confused by what you're trying to do with the wrapper. This is
> what I have for the wrapper in my ruby.i:
>
> int pn_message_save(pn_message_t *msg, char *OUTPUT, size_t *OUTPUT_SIZE);
> %ignore pn_message_save;
With the above the error happens at 65 characters. I took out my wrapper
and ran the test again and it crashes at 65 characters every time.
> I believe this is all that is necessary since the standard typemaps should
> pick up on OUTPUT and OUTPUT_SIZE and just do the right thing.
What I would like to see in the dynamic languages for this is to not
even have to specify the size for the output buffer; i.e., just call:
Qpid::Proton.get_message_data(msg)
and get the data back as one lump.
> Regardless, I think the cause for the issue is that your wrapper function
> is assuming pn_message_save returns a signed size, but actually it returns
> an int that is 0 on success or an error code on failure:
>
> int pn_message_save(pn_message_t *message, char *data, size_t *size);
>
> On success the size of the output data is stored into *size. (I'll add some
> API-doc for this.)
>
> As an aside in looking at this I noticed that there is some duplication
> between the php, ruby, and python files that could be consolidated into the
> cproton.i. I think this particular declaration is part of that duplication
> unless there is some reason I'm missing that it has to be different for
> ruby.
Agreed. Also, we should make sure to remove having the individual
languages include them relatively (i.e., %include "../cproton.i") and
instead place the shared files in the include directory.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
Re: pn_message_save_data interesting problem...
Posted by Rafael Schloming <rh...@alum.mit.edu>.
On Thu, Aug 23, 2012 at 3:27 PM, Darryl L. Pierce <dp...@redhat.com>wrote:
> In working on the stable Ruby APIs on top of Swig I've hit an
> interesting problem with this API.
>
> The stable API for this is Qpid::Proton::get_message_data(msg, size)
> and returns an array that is the size of the string and the string
> itself, respectively. The swig wrapper currently is:
>
> %rename (pn_message_save) wrap_pn_message_save;
> %inline %{
> int wrap_pn_message_save(pn_message_t *msg, char *OUTPUT, size_t
> *OUTPUT_SIZE) {
> ssize_t sz = pn_message_save(msg, OUTPUT, OUTPUT_SIZE);
>
> if (sz < 0) *OUTPUT_SIZE = 0;
> return *OUTPUT_SIZE;
> }
> %}
> %ignore pn_message_save;
>
> One test I've written is to generate random strings of various lengths,
> throwing them at the message and then getting them back to verify they
> were saved properly. And what I'm seeing happening is that, at certain
> lengths (2 specifically), the APIs fail.
>
> Initially, when the length hits either 129 or 257 bytes, the string
> returned does not match the string submitted. If it gets past 129 then
> it ALWAYS fails at 257 bytes. And if I tell it to ignore anything before
> 257 then it always fails at 513.
>
> Not sure on first glance what's causing the problem.
>
I'm a little confused by what you're trying to do with the wrapper. This is
what I have for the wrapper in my ruby.i:
int pn_message_save(pn_message_t *msg, char *OUTPUT, size_t *OUTPUT_SIZE);
%ignore pn_message_save;
I believe this is all that is necessary since the standard typemaps should
pick up on OUTPUT and OUTPUT_SIZE and just do the right thing.
Regardless, I think the cause for the issue is that your wrapper function
is assuming pn_message_save returns a signed size, but actually it returns
an int that is 0 on success or an error code on failure:
int pn_message_save(pn_message_t *message, char *data, size_t *size);
On success the size of the output data is stored into *size. (I'll add some
API-doc for this.)
As an aside in looking at this I noticed that there is some duplication
between the php, ruby, and python files that could be consolidated into the
cproton.i. I think this particular declaration is part of that duplication
unless there is some reason I'm missing that it has to be different for
ruby.
--Rafael
Re: pn_message_save_data interesting problem...
Posted by William Henry <wh...@redhat.com>.
Looks like a word boundary issue. 128+1, 256+1 and 512+1
Some sanity checks (asserts) would help figure it out.
----- Original Message -----
> In working on the stable Ruby APIs on top of Swig I've hit an
> interesting problem with this API.
>
> The stable API for this is Qpid::Proton::get_message_data(msg, size)
> and returns an array that is the size of the string and the string
> itself, respectively. The swig wrapper currently is:
>
> %rename (pn_message_save) wrap_pn_message_save;
> %inline %{
> int wrap_pn_message_save(pn_message_t *msg, char *OUTPUT, size_t
> *OUTPUT_SIZE) {
> ssize_t sz = pn_message_save(msg, OUTPUT, OUTPUT_SIZE);
>
> if (sz < 0) *OUTPUT_SIZE = 0;
> return *OUTPUT_SIZE;
> }
> %}
> %ignore pn_message_save;
>
> One test I've written is to generate random strings of various
> lengths,
> throwing them at the message and then getting them back to verify
> they
> were saved properly. And what I'm seeing happening is that, at
> certain
> lengths (2 specifically), the APIs fail.
>
> Initially, when the length hits either 129 or 257 bytes, the string
> returned does not match the string submitted. If it gets past 129
> then
> it ALWAYS fails at 257 bytes. And if I tell it to ignore anything
> before
> 257 then it always fails at 513.
>
> Not sure on first glance what's causing the problem.
>
> --
> Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
>
>