Archiving an empty container
Dear All, There is an issue with GiNaC archiving an empty container. This is illustrated by the code included below. The archived empty list z is read in the expression z1 as a list with one element. Also such code can crashes id an empty container is archived first. The reason is in lines 215-216 of container.h: here GiNaC got first=last for both: empty containers and containers with one element. I propose the attached patch, which modifies the return value of archive_node::find_last() (called at line 216 of container.h) by 1. All other calls of archive_node::find_last() are revised to agree with the change. Best wishes, Vladimir -- Vladimir V. Kisil http://www.maths.leeds.ac.uk/~kisilv/ Book: Geometry of Mobius Transformations http://goo.gl/EaG2Vu Software: Geometry of cycles http://moebinv.sourceforge.net/ Jupyter (Colab): https://github.com/vvkisil/MoebInv-notebooks Jupyter (CodeOcean): http://doi.org/10.24433/CO.9934595.v2 ====================================== #include <iostream> #include <fstream> #include <stdexcept> #include <ginac/ginac.h> using namespace std; using namespace GiNaC; int main(int argc, char** argv) { ex x = lst{1,2}, y = lst{3}, z = lst{}; GiNaC::archive A; A.archive_ex(x, "2elem"); A.archive_ex(y, "1elem"); A.archive_ex(z, "empty"); ofstream out("/tmp/test.gar", ios::binary); out << A; out.flush(); out.close(); GiNaC::archive A1; std::ifstream ifs("/tmp/test.gar", std::ifstream::in | ios::binary); ifs >> A1; ex x1 = A1.unarchive_ex(lst{}, "2elem") ; ex y1 = A1.unarchive_ex(lst{}, "1elem") ; ex z1 = A1.unarchive_ex(lst{}, "empty") ; x1.dbgprint(); // -> {1,2} y1.dbgprint(); // -> {3} z1.dbgprint(); // -> {1} return 0; }
Hi Vladimir, On 09.09.19 16:07, Vladimir V. Kisil wrote:
There is an issue with GiNaC archiving an empty container. This is illustrated by the code included below. The archived empty list z is read in the expression z1 as a list with one element. Also such code can crashes id an empty container is archived first.
The reason is in lines 215-216 of container.h: here GiNaC got first=last for both: empty containers and containers with one element.
Thanks a lot for researching and reporting this!
I propose the attached patch, which modifies the return value of archive_node::find_last() (called at line 216 of container.h) by 1. All other calls of archive_node::find_last() are revised to agree with the change.
I'm not convinced that changing archive_node::find_last(name) to return an iterator to a property beyond the one searched for is the right fix. It might break existing code. Also, it's not intuitive in the presence of archive_node::find_first(name). This is not like T::begin() and T::end() - notice the different naming! Would it not be better to add something to the interface of class archive_node, like an empty() function or so? All my best, -richy. -- Richard B. Kreckel <https://in.terlu.de/~kreckel/>
Dear Richard, Thanks for commenting on the previous patch, I appreciate the point on preserving return values of public methods. I am attaching an another patch using an additional method. It is as close to the standard checks (it != list.end()) as I can imagine. Best wishes, Vladimir -- Vladimir V. Kisil http://www.maths.leeds.ac.uk/~kisilv/ Book: Geometry of Mobius Transformations http://goo.gl/EaG2Vu Software: Geometry of cycles http://moebinv.sourceforge.net/ Jupyter (Colab): https://github.com/vvkisil/MoebInv-notebooks Jupyter (CodeOcean): http://doi.org/10.24433/CO.9934595.v2
On Tue, 10 Sep 2019 10:27:16 +0200, "Richard B. Kreckel" <kreckel@in.terlu.de> said:
RK> Hi Vladimir, On 09.09.19 16:07, Vladimir V. Kisil wrote: >> There is an issue with GiNaC archiving an empty container. This >> is illustrated by the code included below. The archived empty >> list z is read in the expression z1 as a list with one >> element. Also such code can crashes id an empty container is >> archived first. >> >> The reason is in lines 215-216 of container.h: here GiNaC got >> first=last for both: empty containers and containers with one >> element. RK> Thanks a lot for researching and reporting this! >> I propose the attached patch, which modifies the return value of >> archive_node::find_last() (called at line 216 of container.h) by >> 1. All other calls of archive_node::find_last() are revised to >> agree with the change. RK> I'm not convinced that changing archive_node::find_last(name) to RK> return an iterator to a property beyond the one searched for is RK> the right fix. RK> It might break existing code. Also, it's not intuitive in the RK> presence of archive_node::find_first(name). This is not like RK> T::begin() and T::end() - notice the different naming! RK> Would it not be better to add something to the interface of RK> class archive_node, like an empty() function or so? RK> All my best, -richy. -- Richard B. Kreckel RK> <https://in.terlu.de/~kreckel/> RK> _______________________________________________ GiNaC-devel RK> mailing list GiNaC-devel@ginac.de RK> https://www.cebix.net/mailman/listinfo/ginac-devel
Dear Vladimir, On 10.09.19 12:19, Vladimir V. Kisil wrote:
Thanks for commenting on the previous patch, I appreciate the point on preserving return values of public methods.
I am attaching an another patch using an additional method. It is as close to the standard checks (it != list.end()) as I can imagine.
I'm having trouble with your patches. I tried the sample program you sent initially. Without any patch applied, it prints: {1,2} {3} {1} With your first patch applied, it prints: {1,2,1} {3,{1,2,1}} {1} With your second patch applied, it prints: {1,2} {3} {1} Shouldn't it print {1,2} {3} {} when it's patched? Am I doing something wrong? -richy.
On 22.09.19 18:54, Richard B. Kreckel wrote:
Am I doing something wrong?
Rats! It works fine on another machine. This is obnoxious. Anyway, I'm pushing a slightly different patch. Feel free to test! Thanks a lot! -richy. -- Richard B. Kreckel <https://in.terlu.de/~kreckel/>
On Sun, 22 Sep 2019 19:23:41 +0200, "Richard B. Kreckel" <kreckel@in.terlu.de> said:
RK> On 22.09.19 18:54, Richard B. Kreckel wrote: RK> Anyway, I'm pushing a slightly different patch. Feel free to RK> test! Yes, the latest Git version works fine for me! -- Vladimir V. Kisil http://www.maths.leeds.ac.uk/~kisilv/ Book: Geometry of Mobius Transformations http://goo.gl/EaG2Vu Software: Geometry of cycles http://moebinv.sourceforge.net/ Jupyter (Colab): https://github.com/vvkisil/MoebInv-notebooks Jupyter (CodeOcean): http://doi.org/10.24433/CO.9934595.v3
participants (2)
-
Richard B. Kreckel
-
Vladimir V. Kisil