From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17582 invoked by alias); 29 Apr 2004 23:32:31 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 17546 invoked from network); 29 Apr 2004 23:32:30 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 29 Apr 2004 23:32:30 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i3TNWUKG002411 for ; Thu, 29 Apr 2004 19:32:30 -0400 Received: from zenia.home.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i3TNWSv22441; Thu, 29 Apr 2004 19:32:28 -0400 To: Joel Brobecker Cc: Daniel Jacobowitz , Eli Zaretskii , gdb-patches@sources.redhat.com Subject: Re: [RFC/dwarf-2] Add support for included files References: <20040413052021.GA1173@gnat.com> <20040415221338.GF1564@gnat.com> <20040416230825.GK22414@gnat.com> From: Jim Blandy Date: Thu, 29 Apr 2004 23:32:00 -0000 In-Reply-To: <20040416230825.GK22414@gnat.com> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2004-04/txt/msg00696.txt.bz2 Joel Brobecker writes: > Here is an updated version of the previous patch, adapted to fit in > the current dwarf2read.c after Daniel's changes, and with comments > added. It's actually slightly smaller thanks to Daniel's changes > :-). Great! I actually have still more comments: - Rather than passing 'line_offset' by reference to read_partial_die, could you add a line_offset field to 'struct partial_die_info, and have read_partial_die set that? That would seem more consistent with the rest of what read_partial_die does. Then 'dwarf2_build_include_psymtabs' should take a pointer to the CU die. That's assuming Daniel doesn't see any problem enlarging partial_die_info. If he does, maybe we could add the fields to 'struct dwarf2_cu' instead: it's a little odd to have 'read_partial_die' set the cu's fields directly, but not horribly so, as long as the die's tag is DW_TAG_compile_unit. - Is it really okay to pass NULL to dwarf_decode_lines for comp_dir? Won't the filenames of the partial symbol tables be different from those of the symbol tables? To fix this, read_partial_die would have to check for DW_AT_comp_dir attributes, too. - It seems to me the file_is_included array should be in struct line_header, with add_file_name managing its allocation / initialization in parallel with file_names. It's already the case that that structure is mutated by the act of processing the line number program, so I don't see that it's a serious change to the usage patterns of that structure type. - I'd rather see the call to dwarf2_build_include_psymtabs before the assignment to info_ptr, not after. Not that it matters much. - Thanks for fixing the comments on dwarf_decode_lines --- especially the references to arguments that had been missing for some time now.