[GNC-dev] Bug # 796687

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[GNC-dev] Bug # 796687

Alex Aycinena-2
Geert,

I have been investigating bug # 796687 (Tax Entity name and type for an
account won't work under "Tax Reporting Options" in Gnucash 3.2) and it
seems that your commit 4053f2ca5331b345f04f75022658d254c36bdd15 on Mar 31,
2018, has caused book string options not to work. I ran gnucash under gdb
and got the following:

qof_book_set_string_option (book=0xb49af0,
    opt_name=0x7fffefcd57e9 "book/tax_US/type", opt_val=0x2600640 "F1040")
    at
/home/gnucash-dev/gitcheckouts/gnucash-acct-maint-latest/libgnucash/engine/qofbook.cpp:1163
1163        qof_book_begin_edit(book);

going into the function. The book, opt_name and opt_val all seem fine but
the KVP is not saved. When I checked-out the commit just prior to yours, it
seems to work fine: the data is saved and retrieved as expected. After your
commit, it seems to stop working.

I tried to analyze your commit but I'm not sure I understand it. Perhaps
you could help me understand it. The funny thing is, the unit test seems to
pass.

Can you help me figure out why it is not working?

Thanks,

Alex
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: [GNC-dev] Bug # 796687

Geert Janssens-4
On Sunday, January 6, 2019 4:55:09 AM CET Alex Aycinena wrote:

> Geert,
>
> I have been investigating bug # 796687 (Tax Entity name and type for an
> account won't work under "Tax Reporting Options" in Gnucash 3.2) and it
> seems that your commit 4053f2ca5331b345f04f75022658d254c36bdd15 on Mar 31,
> 2018, has caused book string options not to work. I ran gnucash under gdb
> and got the following:
>
> qof_book_set_string_option (book=0xb49af0,
>     opt_name=0x7fffefcd57e9 "book/tax_US/type", opt_val=0x2600640 "F1040")
>     at
> /home/gnucash-dev/gitcheckouts/gnucash-acct-maint-latest/libgnucash/engine/q
> ofbook.cpp:1163 1163        qof_book_begin_edit(book);
>
> going into the function. The book, opt_name and opt_val all seem fine but
> the KVP is not saved. When I checked-out the commit just prior to yours, it
> seems to work fine: the data is saved and retrieved as expected. After your
> commit, it seems to stop working.
>
> I tried to analyze your commit but I'm not sure I understand it. Perhaps
> you could help me understand it. The funny thing is, the unit test seems to
> pass.
>
> Can you help me figure out why it is not working?
>
> Thanks,
>
> Alex

Alex,

I'm not at my development machine until Wednesday. I'll get back to this later
this week.

Geert



_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: [GNC-dev] Bug # 796687

Alex Aycinena-2
Geert,

On Sun, Jan 6, 2019 at 8:11 AM Geert Janssens <[hidden email]>
wrote:

> On Sunday, January 6, 2019 4:55:09 AM CET Alex Aycinena wrote:
> > Geert,
> >
> > I have been investigating bug # 796687 (Tax Entity name and type for an
> > account won't work under "Tax Reporting Options" in Gnucash 3.2) and it
> > seems that your commit 4053f2ca5331b345f04f75022658d254c36bdd15 on Mar
> 31,
> > 2018, has caused book string options not to work. I ran gnucash under gdb
> > and got the following:
> >
> > qof_book_set_string_option (book=0xb49af0,
> >     opt_name=0x7fffefcd57e9 "book/tax_US/type", opt_val=0x2600640
> "F1040")
> >     at
> >
> /home/gnucash-dev/gitcheckouts/gnucash-acct-maint-latest/libgnucash/engine/q
> > ofbook.cpp:1163 1163        qof_book_begin_edit(book);
> >
> > going into the function. The book, opt_name and opt_val all seem fine but
> > the KVP is not saved. When I checked-out the commit just prior to yours,
> it
> > seems to work fine: the data is saved and retrieved as expected. After
> your
> > commit, it seems to stop working.
> >
> > I tried to analyze your commit but I'm not sure I understand it. Perhaps
> > you could help me understand it. The funny thing is, the unit test seems
> to
> > pass.
> >
> > Can you help me figure out why it is not working?
> >
> > Thanks,
> >
> > Alex
>
> Alex,
>
> I'm not at my development machine until Wednesday. I'll get back to this
> later
> this week.
>
> Geert
>
>
OK. Thanks,

Alex
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: [GNC-dev] Bug # 796687

Alex Aycinena-2
Geert,

On Sun, Jan 6, 2019 at 9:17 AM Alex Aycinena <[hidden email]>
wrote:

> Geert,
>
> On Sun, Jan 6, 2019 at 8:11 AM Geert Janssens <[hidden email]>
> wrote:
>
>> On Sunday, January 6, 2019 4:55:09 AM CET Alex Aycinena wrote:
>> > Geert,
>> >
>> > I have been investigating bug # 796687 (Tax Entity name and type for an
>> > account won't work under "Tax Reporting Options" in Gnucash 3.2) and it
>> > seems that your commit 4053f2ca5331b345f04f75022658d254c36bdd15 on Mar
>> 31,
>> > 2018, has caused book string options not to work. I ran gnucash under
>> gdb
>> > and got the following:
>> >
>> > qof_book_set_string_option (book=0xb49af0,
>> >     opt_name=0x7fffefcd57e9 "book/tax_US/type", opt_val=0x2600640
>> "F1040")
>> >     at
>> >
>> /home/gnucash-dev/gitcheckouts/gnucash-acct-maint-latest/libgnucash/engine/q
>> > ofbook.cpp:1163 1163        qof_book_begin_edit(book);
>> >
>> > going into the function. The book, opt_name and opt_val all seem fine
>> but
>> > the KVP is not saved. When I checked-out the commit just prior to
>> yours, it
>> > seems to work fine: the data is saved and retrieved as expected. After
>> your
>> > commit, it seems to stop working.
>> >
>> > I tried to analyze your commit but I'm not sure I understand it. Perhaps
>> > you could help me understand it. The funny thing is, the unit test
>> seems to
>> > pass.
>> >
>> > Can you help me figure out why it is not working?
>> >
>> > Thanks,
>> >
>> > Alex
>>
>> Alex,
>>
>> I'm not at my development machine until Wednesday. I'll get back to this
>> later
>> this week.
>>
>> Geert
>>
>>
> OK. Thanks,
>
> Alex
>

The following change seems to fix it:

diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp
index c0cb043ba..fa67f5b65 100644
--- a/libgnucash/engine/qofbook.cpp
+++ b/libgnucash/engine/qofbook.cpp
@@ -1164,9 +1164,9 @@ qof_book_set_string_option(QofBook* book, const char*
opt_name, const char* opt_
     auto frame = qof_instance_get_slots(QOF_INSTANCE(book));
     auto opt_path = opt_name_to_path(opt_name);
     if (opt_val && (*opt_val != '\0'))
-        delete frame->set(opt_path, new KvpValue(g_strdup(opt_val)));
+        delete frame->set_path(opt_path, new KvpValue(g_strdup(opt_val)));
     else
-        delete frame->set(opt_path, nullptr);
+        delete frame->set_path(opt_path, nullptr);
     qof_instance_set_dirty (QOF_INSTANCE (book));
     qof_book_commit_edit(book);
 }

If you agree with the change, I will commit it.

Thanks,

Alex
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: [GNC-dev] Bug # 796687

Geert Janssens-4
Op dinsdag 8 januari 2019 01:31:58 CET schreef Alex Aycinena:

> Geert,
>
> On Sun, Jan 6, 2019 at 9:17 AM Alex Aycinena <[hidden email]>
>
> wrote:
> > Geert,
> >
> > On Sun, Jan 6, 2019 at 8:11 AM Geert Janssens <[hidden email]>
> >
> > wrote:
> >> On Sunday, January 6, 2019 4:55:09 AM CET Alex Aycinena wrote:
> >> > Geert,
> >> >
> >> > I have been investigating bug # 796687 (Tax Entity name and type for an
> >> > account won't work under "Tax Reporting Options" in Gnucash 3.2) and it
> >> > seems that your commit 4053f2ca5331b345f04f75022658d254c36bdd15 on Mar
> >>
> >> 31,
> >>
> >> > 2018, has caused book string options not to work. I ran gnucash under
> >>
> >> gdb
> >>
> >> > and got the following:
> >> >
> >> > qof_book_set_string_option (book=0xb49af0,
> >> >
> >> >     opt_name=0x7fffefcd57e9 "book/tax_US/type", opt_val=0x2600640
> >>
> >> "F1040")
> >>
> >> >     at
> >>
> >> /home/gnucash-dev/gitcheckouts/gnucash-acct-maint-latest/libgnucash/engin
> >> e/q>>
> >> > ofbook.cpp:1163 1163        qof_book_begin_edit(book);
> >> >
> >> > going into the function. The book, opt_name and opt_val all seem fine
> >>
> >> but
> >>
> >> > the KVP is not saved. When I checked-out the commit just prior to
> >>
> >> yours, it
> >>
> >> > seems to work fine: the data is saved and retrieved as expected. After
> >>
> >> your
> >>
> >> > commit, it seems to stop working.
> >> >
> >> > I tried to analyze your commit but I'm not sure I understand it.
> >> > Perhaps
> >> > you could help me understand it. The funny thing is, the unit test
> >>
> >> seems to
> >>
> >> > pass.
> >> >
> >> > Can you help me figure out why it is not working?
> >> >
> >> > Thanks,
> >> >
> >> > Alex
> >>
> >> Alex,
> >>
> >> I'm not at my development machine until Wednesday. I'll get back to this
> >> later
> >> this week.
> >>
> >> Geert
> >
> > OK. Thanks,
> >
> > Alex
>
> The following change seems to fix it:
>
> diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp
> index c0cb043ba..fa67f5b65 100644
> --- a/libgnucash/engine/qofbook.cpp
> +++ b/libgnucash/engine/qofbook.cpp
> @@ -1164,9 +1164,9 @@ qof_book_set_string_option(QofBook* book, const char*
> opt_name, const char* opt_
>      auto frame = qof_instance_get_slots(QOF_INSTANCE(book));
>      auto opt_path = opt_name_to_path(opt_name);
>      if (opt_val && (*opt_val != '\0'))
> -        delete frame->set(opt_path, new KvpValue(g_strdup(opt_val)));
> +        delete frame->set_path(opt_path, new KvpValue(g_strdup(opt_val)));
>      else
> -        delete frame->set(opt_path, nullptr);
> +        delete frame->set_path(opt_path, nullptr);
>      qof_instance_set_dirty (QOF_INSTANCE (book));
>      qof_book_commit_edit(book);
>  }
>
> If you agree with the change, I will commit it.
>
> Thanks,
>
> Alex

Alex,

That patch looks right to me. The difference between set and set_path appears
to be that set_path creates missing frames where set will do nothing in case
of missing frames. So if for whatever reason (first use, reset,...) a certain
string option is not set it will not be created using "set". Perhaps "set"
would better have been named "update" and "set_path" "create_or_update". Oh,
well.

Good detective work!

Geert


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel