Add necessary unref to accessor.c and report.c #1334
Conversation
HashidaTKS
force-pushed
the
add_unref_to_select
branch
3 times, most recently
from
b15bba8
to
d7a8741
Compare
Add a test
HashidaTKS
force-pushed
the
add_unref_to_select
branch
from
d7a8741
to
5d49b2b
Compare
lib/report.c
Outdated
| @@ -87,6 +87,9 @@ grn_report_table(grn_ctx *ctx, | |||
| GRN_TEXT_PUT(ctx, &description, name, name_size); | |||
| GRN_TEXT_PUTS(ctx, &description, ">"); | |||
| } | |||
| if (target != table) { | |||
| grn_obj_unref(ctx, target); | |||
There was a problem hiding this comment.
This is invalid.
We use target after this grn_obj_unref for target = grn_ctx_at(ctx, target->header.domain). If target is closed by this grn_obj_unref(), target->header.domain touches invalid memory.
There was a problem hiding this comment.
Right, I have fixed it.
| #@add-ignore-log-pattern /\A\[io\]/ | ||
| log_level --level dump | ||
| select Items \ | ||
| --column[price_with_tax].stage initial \ |
There was a problem hiding this comment.
Could you use columns instead of column because column is deprecated?
There was a problem hiding this comment.
I have fixed it.
Modify to use columns in the test
lib/report.c
Outdated
| @@ -65,7 +65,7 @@ grn_report_table(grn_ctx *ctx, | |||
| grn_obj *table) | |||
| { | |||
| grn_obj description; | |||
| grn_obj *target; | |||
| grn_obj *target, *previous_target = NULL; | |||
There was a problem hiding this comment.
Could you declare one variable per line?
There was a problem hiding this comment.
I have fixed it.
lib/report.c
Outdated
| if (previous_target && previous_target != table) { | ||
| grn_obj_unref(ctx, previous_target); | ||
| } | ||
| previous_target = target; |
There was a problem hiding this comment.
Could you move this to for (...) to indicating that this is update in each loop?
There was a problem hiding this comment.
I have fixed it.
|
I have noticed that |
|
It makes sense. |
Related: #1333, #1330
When executing
grn_accessor_executeorgrn_report_table, a referred object is not un-referred in some cases.I have added necessary
unrefs for those.Please see the added
reference_count.testfor reproducing the issue.This affect to #1330.