boostorg / property_tree

Boost.org property_tree module
http://boost.org/libs/property_tree
55 stars 92 forks source link

handle xml writing of null keys #87

Closed brchrisman closed 3 years ago

brchrisman commented 3 years ago

boost::property_tree can have nulls as keys. In particular, json read typically generates child nodes with null keys as part of array processing. Place a nonce (3cb6534e-d358-4705-9e74-fee06453661e) as a unique tag name for null keys in write_xml.

madmongo1 commented 3 years ago

Thank you for the contribution.

Is there any way we can add a test to:

madmongo1 commented 3 years ago

I currently see test failures in C++03:

gcc.compile.c++ bin.v2/libs/property_tree/test/test_multi_module1.test/gcc-9/debug/cxxstd-03-iso/link-static/threading-multi/visibility-hidden/test_multi_module2.o
In file included from ./boost/property_tree/xml_parser.hpp:15,
                 from libs/property_tree/test/test_multi_module2.cpp:14:
./boost/property_tree/detail/xml_parser_write.hpp: In function ‘void boost::property_tree::xml_parser::write_xml_element(std::basic_ostream<typename Ptree::key_type::value_type>&, const typename Ptree::key_type&, const Ptree&, int, const boost::property_tree::xml_parser::xml_writer_settings<typename Ptree::key_type>&)’:
./boost/property_tree/detail/xml_parser_write.hpp:75:31: error: ‘R’ was not declared in this scope
   75 |         const char* nullkey = R"nullkey(nullkey-3cb6534e-d358-4705-9e74-fee06453661e)nullkey";
      |                               ^
brchrisman commented 3 years ago

Thank you for the contribution.

Is there any way we can add a test to:

  • demonstrate no legacy impact
  • test the problem that this fixes
  • demonstrate that it works in C++03

Thanks for the feedback, I will look through the tests and move the string literal to c++03 compatibility.

brchrisman commented 3 years ago

Fixed c++03 compatibility. Added ptree key-nulling on xml ingestion for the same nullkey- nonce and included a test with the 'total key size' showing that the nonce is not included in the keys. I don't have the exact build tree, but checked those values on an environment with this change integrated.

brchrisman commented 3 years ago

I'm not sure how to trigger another run of automated build/testing or whether I can, but it should be ready. thanks, Brian

brchrisman commented 3 years ago

Pushed fixes here and was wondering whether the automated testing can be run to verify. i'm not sure how to signal that the PR has been changed and/or is ready.

On Tue, Jun 8, 2021 at 2:05 AM Richard Hodges @.***> wrote:

I currently see test failures in C++03:

gcc.compile.c++ bin.v2/libs/property_tree/test/test_multi_module1.test/gcc-9/debug/cxxstd-03-iso/link-static/threading-multi/visibility-hidden/test_multi_module2.o

In file included from ./boost/property_tree/xml_parser.hpp:15,

             from libs/property_tree/test/test_multi_module2.cpp:14:

./boost/property_tree/detail/xml_parser_write.hpp: In function ‘void boost::property_tree::xml_parser::write_xml_element(std::basic_ostream&, const typename Ptree::key_type&, const Ptree&, int, const boost::property_tree::xml_parser::xml_writer_settings&)’:

./boost/property_tree/detail/xml_parser_write.hpp:75:31: error: ‘R’ was not declared in this scope

75 | const char* nullkey = R"nullkey(nullkey-3cb6534e-d358-4705-9e74-fee06453661e)nullkey";

  |                               ^

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/boostorg/property_tree/pull/87#issuecomment-856596035, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFVWM2XRT4RVMCJDKCSOADTRXML3ANCNFSM46IZCHYA .