On Mon, 11 Feb 2013, Danny Al-Gaaf wrote:
> Am 10.02.2013 06:57, schrieb Sage Weil:
> > On Thu, 7 Feb 2013, Danny Al-Gaaf wrote:
> >> Fix "variable length array of non-POD element type" errors caused by
> >> using librados::ObjectWriteOperation VLAs. (-Wvla)
> >>
> >> Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@xxxxxxxxx>
> >> ---
> >> src/key_value_store/kv_flat_btree_async.cc | 14 +++++++-------
> >> 1 file changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/src/key_value_store/kv_flat_btree_async.cc b/src/key_value_store/kv_flat_btree_async.cc
> >> index 96c6cb0..4342e70 100644
> >> --- a/src/key_value_store/kv_flat_btree_async.cc
> >> +++ b/src/key_value_store/kv_flat_btree_async.cc
> >> @@ -1119,9 +1119,9 @@ int KvFlatBtreeAsync::cleanup(const index_data &idata, const int &errno) {
> >> //all changes were created except for updating the index and possibly
> >> //deleting the objects. roll forward.
> >> vector<pair<pair<int, string>, librados::ObjectWriteOperation*> > ops;
> >> - librados::ObjectWriteOperation owos[idata.to_delete.size() + 1];
> >> + vector<librados::ObjectWriteOperation*> owos(idata.to_delete.size() + 1);
> >
> > I haven't read much of the surrounding code, but from what is included
> > here I don't think this is equivalent... these are just null pointers
> > initially, and so
> >
> >> for (int i = 0; i <= (int)idata.to_delete.size(); ++i) {
> >> - ops.push_back(make_pair(pair<int, string>(0, ""), &owos[i]));
> >> + ops.push_back(make_pair(pair<int, string>(0, ""), owos[i]));
> >
> > this doesn't do anything useful... owos[i] may as well be NULL. Why not
> > make it
> >
> > vector<librados::ObjectWriteOperation> owos(...)
> >
> > ?
>
> Because this would lead to a linker error:
>
> kv_flat_btree_async.o: In function `void
> std::__uninitialized_fill_n<false>::__uninit_fill_n<librados::ObjectWriteOperation*,
> unsigned long,
> librados::ObjectWriteOperation>(librados::ObjectWriteOperation*,
> unsigned long, librados::ObjectWriteOperation const&)':
> /usr/bin/../lib64/gcc/x86_64-suse-linux/4.7/../../../../include/c++/4.7/bits/stl_uninitialized.h:188:
> undefined reference to
> `librados::ObjectOperation::ObjectOperation(librados::ObjectOperation
> const&)'
> /usr/bin/../lib64/gcc/x86_64-suse-linux/4.7/../../../../include/c++/4.7/bits/stl_uninitialized.h:188:
> undefined reference to
> `librados::ObjectOperation::ObjectOperation(librados::ObjectOperation
> const&)'
>
>
> Because in src/include/rados/librados.hpp
> librados::ObjectOperation::ObjectOperation(librados::ObjectOperation
> const&) was is defined, but not implemented in the librados.cc.
>
> Not sure if removing ObjectOperation(librados::ObjectOperation const&)
> is the way to go here.
Oh, I see... yeah, we shouldn't remove that. Probably we should
restructure the code to use a list<>, which doesn't require a copy
constructor or assignment operator.
Note that this particular code shouldn't hold up the rest of the patches,
since it's not being used by anything (yet!).
sage
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
