> Hi, I don't fully understand what this patch is intended to do. Why is it
> necessary?
It
- fixes a bug where errors parsing the file do not terminate the processing
- fixes a small memory leak
- simplifies the code
Otherwise extending this code for DWARF-5 in a later patch would be difficult
how to code it with these existing issues.
> commit 0da448c337d481f1c50af212246ceb213a7d80cc (HEAD -> master) Author: Jan
> Kratochvil ***@***.***> Date: Mon Aug 17 21:46:47 2020 +0200 debugedit: Fix
> handling in caller for errors in called read_dwarf2_line. diff --git
> a/tools/debugedit.c b/tools/debugedit.c index 8e85847d1..ff72759ca 100644 ---
> a/tools/debugedit.c +++ b/tools/debugedit.c @@ -1155,7 +1155,7 @@
> get_line_table (DSO *dso, size_t off, struct line_table **table) if
> (lines->table[i].old_idx == off) { *table = &lines->table[i]; - return false;
> + return true; }
This is really a mess, couldn't you learn how to do a patch review?
> This breaks the contract of the function, which says:
I forgot to update the function comment, fixed.
> The intention is to let the caller know whether the line_table has already
> been handled or not earlier. It makes sure read_dwarf2_line doesn't try
> parsing the same line table multiple times even if it is referenced multiple
> times by different CUs.
This
- is no longer possible as for DWARF-5 one needs to parse the same line table
in multiple phases
- therefore this check is moved later into:
```
if (phase <= table->phase_done)
return true;
table->phase_done = phase;
```
> Returning true in this case might make read_dwarf2_line parse and replace the
> same line table multiple times.
I do not think it matters and this gets fixed in later patch.
> Again this changes the documented contract of the function which says:
I have updated the function comment.
> Why do you need to call read_dwarf2_line in all phases here?
Because DWARF-5 .debug_line rewrite requires that. I have moved this change to
the DWARF-5 later patch.
> This seems to fix a minor memory leak. Looks OK.
This would not work without the other fixes.
--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1332#issuecomment-731388373_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
