| Message ID | 1360266123-28972-2-git-send-email-danny.al-gaaf@bisect.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@bisect.de> > --- > 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(...) ? > } > set_up_ops(vector<object_data>(), > vector<object_data>(), &ops, idata, &err); > @@ -1883,23 +1883,23 @@ int KvFlatBtreeAsync::set_many(const map<string, bufferlist> &in_map) { > to_create[to_create.size() - 1].max_kdata = > to_delete[to_delete.size() - 1].max_kdata; > > - librados::ObjectWriteOperation owos[2 + 2 * to_delete.size() > - + to_create.size()]; > + vector<librados::ObjectWriteOperation*> owos(2 + 2 * to_delete.size() > + + to_create.size()); Same thing here... > vector<pair<pair<int, string>, librados::ObjectWriteOperation*> > ops; > > > index_data idata; > - set_up_prefix_index(to_create, to_delete, &owos[0], &idata, &err); > + set_up_prefix_index(to_create, to_delete, owos[0], &idata, &err); > > if (verbose) cout << "finished making to_create and to_delete. " > << std::endl; > > ops.push_back(make_pair( > pair<int, string>(ADD_PREFIX, index_name), > - &owos[0])); > + owos[0])); > for (int i = 1; i < 2 + 2 * (int)to_delete.size() + (int)to_create.size(); > i++) { > - ops.push_back(make_pair(make_pair(0,""), &owos[i])); > + ops.push_back(make_pair(make_pair(0,""), owos[i])); > } > > set_up_ops(to_create, to_delete, &ops, idata, &err); > -- > 1.8.1.2 > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@bisect.de> >> --- >> 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. Danny -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@bisect.de> > >> --- > >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); 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])); } set_up_ops(vector<object_data>(), vector<object_data>(), &ops, idata, &err); @@ -1883,23 +1883,23 @@ int KvFlatBtreeAsync::set_many(const map<string, bufferlist> &in_map) { to_create[to_create.size() - 1].max_kdata = to_delete[to_delete.size() - 1].max_kdata; - librados::ObjectWriteOperation owos[2 + 2 * to_delete.size() - + to_create.size()]; + vector<librados::ObjectWriteOperation*> owos(2 + 2 * to_delete.size() + + to_create.size()); vector<pair<pair<int, string>, librados::ObjectWriteOperation*> > ops; index_data idata; - set_up_prefix_index(to_create, to_delete, &owos[0], &idata, &err); + set_up_prefix_index(to_create, to_delete, owos[0], &idata, &err); if (verbose) cout << "finished making to_create and to_delete. " << std::endl; ops.push_back(make_pair( pair<int, string>(ADD_PREFIX, index_name), - &owos[0])); + owos[0])); for (int i = 1; i < 2 + 2 * (int)to_delete.size() + (int)to_create.size(); i++) { - ops.push_back(make_pair(make_pair(0,""), &owos[i])); + ops.push_back(make_pair(make_pair(0,""), owos[i])); } set_up_ops(to_create, to_delete, &ops, idata, &err);
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@bisect.de> --- src/key_value_store/kv_flat_btree_async.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)