qpid-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alan Conway <acon...@redhat.com>
Subject Re: [Proton-C - 0.12.2] C++ vector subscript out of range in unit tests
Date Thu, 02 Jun 2016 18:18:59 GMT
On Thu, 2016-06-02 at 14:07 -0400, Andrew Stitcher wrote:
> On Thu, 2016-06-02 at 10:29 -0700, Adel Boutros wrote:
> > 
> > Hello,
> > 
> > While compiling the unit tests of Proton-c 0.12.2 on Visual Studio
> > 2008 in
> > Debug mode, the cpp_scalar_test and cpp_value_test are failing with
> > the
> > error: "C++ vector subscript out of range" 
> > After debugging, It seems that we try to cast the first element of
> > a
> > vector
> > and the test has an empty vector which is causing the issue.
> > 
> > The test fails for the following values:
> > 
> >     try { coerce<symbol>(V(binary())); FAIL("binary as symbol"); }
> > catch
> > (conversion_error) {}
> >     try { coerce<binary>(V(symbol())); FAIL("symbol as binary"); }
> > catch
> > (conversion_error) {}
> >     try { get<symbol>(V(std::string())); FAIL("string as symbol");
> > }
> > catch
> > (conversion_error) {}
> >     try { get<std::string>(V(binary())); FAIL("binary as string");
> > }
> > catch
> > (conversion_error) {}
> >     try { get<binary>(V(symbol())); FAIL("symbol as binary"); }
> > catch
> > (conversion_error) {}
> > 
> > string, binary and symbol default constructor will construct an
> > empty
> > vector.
> > 
> > So in types_internal.hpp, we do: pn_bytes_t b = { s.size(),
> > reinterpret_cast<const char*>(&s[0]) };
> > "s" is the empty vector so "s[0]" will throw the exception observed
> > 
> > I have done a workaround  in the attached patch and the test is
> > green
> > now.
> > But I think a clean fix is needed to prevent such error in
> > production
> > mode,
> > no?
> Yeah, I don't think there is much point in shipping that work around.
> The test is functioning as required after all - it is failing when it
> should. So making it pass would be wrong.

Agreed, it's the impl not the test that needs fixing. I'll add a fix
to 
PROTON-1216: c++: proton::coerce<std::string>() should allow conversion
from binary.

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Mime
View raw message