From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128889 invoked by alias); 22 Mar 2017 18:01:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 128873 invoked by uid 89); 22 Mar 2017 18:01:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Mar 2017 18:01:41 +0000 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v2MHwbD8097866 for ; Wed, 22 Mar 2017 14:01:41 -0400 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0a-001b2d01.pphosted.com with ESMTP id 29bv3gxnm6-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 22 Mar 2017 14:01:40 -0400 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 22 Mar 2017 18:01:39 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197) by e06smtp15.uk.ibm.com (192.168.101.145) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 22 Mar 2017 18:01:37 -0000 Received: from d06av24.portsmouth.uk.ibm.com (d06av24.portsmouth.uk.ibm.com [9.149.105.60]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v2MI1bo236634714; Wed, 22 Mar 2017 18:01:37 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EE0F242041; Wed, 22 Mar 2017 18:01:07 +0000 (GMT) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CA6C14203F; Wed, 22 Mar 2017 18:01:07 +0000 (GMT) Received: from ThinkPad (unknown [9.152.212.148]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 22 Mar 2017 18:01:07 +0000 (GMT) Date: Wed, 22 Mar 2017 18:01:00 -0000 From: Philipp Rudo To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: Make ldirname return a std::string In-Reply-To: <1490198646-10469-1-git-send-email-palves@redhat.com> References: <1490198646-10469-1-git-send-email-palves@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17032218-0020-0000-0000-0000032B4DD4 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17032218-0021-0000-0000-000040E9B393 Message-Id: <20170322190135.2d634571@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-22_15:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703220156 X-IsSubscribed: yes X-SW-Source: 2017-03/txt/msg00408.txt.bz2 Hi On Wed, 22 Mar 2017 16:04:06 +0000 Pedro Alves wrote: [...] > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index b3ea52b..4233006 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c [...] > @@ -9122,37 +9130,37 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu) > return cu->producer_is_gcc_lt_4_3; > } > > -static void > -find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu, > - const char **name, const char **comp_dir) > +static file_and_directory > +find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu) > { > + file_and_directory res; > + > /* Find the filename. Do not use dwarf2_name here, since the > filename is not a source language identifier. */ > - *name = dwarf2_string_attr (die, DW_AT_name, cu); > - *comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu); > + res.name = dwarf2_string_attr (die, DW_AT_name, cu); > + res.comp_dir = dwarf2_string_attr (die, DW_AT_comp_dir, cu); > > - if (*comp_dir == NULL > - && producer_is_gcc_lt_4_3 (cu) && *name != NULL > - && IS_ABSOLUTE_PATH (*name)) > + if (res.comp_dir == NULL > + && producer_is_gcc_lt_4_3 (cu) && res.name != NULL > + && IS_ABSOLUTE_PATH (res.name)) > { > - char *d = ldirname (*name); > - > - *comp_dir = d; > - if (d != NULL) > - make_cleanup (xfree, d); > + res.comp_dir_storage = ldirname (res.name); > + res.comp_dir = res.comp_dir_storage.c_str (); > } > - if (*comp_dir != NULL) > + if (res.comp_dir != NULL) Is this check valid? Doesn't std::string.c_str () always return a pointer != NULL? I would check for res.comp_dir_storage.empty (). Otherwise the patch is ok. Philipp > { > /* Irix 6.2 native cc prepends .: to the compilation > directory, get rid of it. */ > - const char *cp = strchr (*comp_dir, ':'); > + const char *cp = strchr (res.comp_dir, ':'); > > - if (cp && cp != *comp_dir && cp[-1] == '.' && cp[1] == '/') > - *comp_dir = cp + 1; > + if (cp && cp != res.comp_dir && cp[-1] == '.' && cp[1] == '/') > + res.comp_dir = cp + 1; > } > > - if (*name == NULL) > - *name = ""; > + if (res.name == NULL) > + res.name = ""; > + > + return res; > } > > /* Handle DW_AT_stmt_list for a compilation unit. > @@ -9262,12 +9270,9 @@ read_file_scope (struct die_info *die, struct > dwarf2_cu *cu) { > struct objfile *objfile = dwarf2_per_objfile->objfile; > struct gdbarch *gdbarch = get_objfile_arch (objfile); > - struct cleanup *back_to = make_cleanup (null_cleanup, 0); > CORE_ADDR lowpc = ((CORE_ADDR) -1); > CORE_ADDR highpc = ((CORE_ADDR) 0); > struct attribute *attr; > - const char *name = NULL; > - const char *comp_dir = NULL; > struct die_info *child_die; > CORE_ADDR baseaddr; > > @@ -9281,7 +9286,7 @@ read_file_scope (struct die_info *die, struct > dwarf2_cu *cu) lowpc = highpc; > lowpc = gdbarch_adjust_dwarf2_addr (gdbarch, lowpc + baseaddr); > > - find_file_and_directory (die, cu, &name, &comp_dir); > + file_and_directory fnd = find_file_and_directory (die, cu); > > prepare_one_comp_unit (cu, die, cu->language); > > @@ -9295,12 +9300,12 @@ read_file_scope (struct die_info *die, struct > dwarf2_cu *cu) if (cu->producer && strstr (cu->producer, "GNU Go > ") != NULL) set_cu_language (DW_LANG_Go, cu); > > - dwarf2_start_symtab (cu, name, comp_dir, lowpc); > + dwarf2_start_symtab (cu, fnd.name, fnd.comp_dir, lowpc); > > /* Decode line number information if present. We do this before > processing child DIEs, so that the line header table is > available for DW_AT_decl_file. */ > - handle_DW_AT_stmt_list (die, cu, comp_dir, lowpc); > + handle_DW_AT_stmt_list (die, cu, fnd.comp_dir, lowpc); > > /* Process all dies in compilation unit. */ > if (die->child != NULL) > @@ -9338,8 +9343,6 @@ read_file_scope (struct die_info *die, struct > dwarf2_cu *cu) dwarf_decode_macros (cu, macro_offset, 0); > } > } > - > - do_cleanups (back_to); > } > > /* TU version of handle_DW_AT_stmt_list for read_type_unit_scope. > @@ -10892,49 +10895,44 @@ open_and_init_dwp_file (void) > { > struct objfile *objfile = dwarf2_per_objfile->objfile; > struct dwp_file *dwp_file; > - char *dwp_name; > - struct cleanup *cleanups = make_cleanup (null_cleanup, 0); > > /* Try to find first .dwp for the binary file before any symbolic > links resolving. */ > > /* If the objfile is a debug file, find the name of the real binary > file and get the name of dwp file from there. */ > + std::string dwp_name; > if (objfile->separate_debug_objfile_backlink != NULL) > { > struct objfile *backlink = > objfile->separate_debug_objfile_backlink; const char > *backlink_basename = lbasename (backlink->original_name); > - char *debug_dirname = ldirname (objfile->original_name); > > - make_cleanup (xfree, debug_dirname); > - dwp_name = xstrprintf ("%s%s%s.dwp", debug_dirname, > - SLASH_STRING, backlink_basename); > + dwp_name = ldirname (objfile->original_name) + SLASH_STRING + > backlink_basename; } > else > - dwp_name = xstrprintf ("%s.dwp", objfile->original_name); > - make_cleanup (xfree, dwp_name); > + dwp_name = objfile->original_name; > + > + dwp_name += ".dwp"; > > - gdb_bfd_ref_ptr dbfd (open_dwp_file (dwp_name)); > + gdb_bfd_ref_ptr dbfd (open_dwp_file (dwp_name.c_str ())); > if (dbfd == NULL > && strcmp (objfile->original_name, objfile_name (objfile)) != > 0) { > /* Try to find .dwp for the binary file after gdb_realpath > resolving. */ > - dwp_name = xstrprintf ("%s.dwp", objfile_name (objfile)); > - make_cleanup (xfree, dwp_name); > - dbfd = open_dwp_file (dwp_name); > + dwp_name = objfile_name (objfile); > + dwp_name += ".dwp"; > + dbfd = open_dwp_file (dwp_name.c_str ()); > } > > if (dbfd == NULL) > { > if (dwarf_read_debug) > - fprintf_unfiltered (gdb_stdlog, "DWP file not found: %s\n", > dwp_name); > - do_cleanups (cleanups); > + fprintf_unfiltered (gdb_stdlog, "DWP file not found: %s\n", > dwp_name.c_str ()); return NULL; > } > dwp_file = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct > dwp_file); dwp_file->name = bfd_get_filename (dbfd.get ()); > dwp_file->dbfd = dbfd.release (); > - do_cleanups (cleanups); > > /* +1: section 0 is unused */ > dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1; > @@ -10958,7 +10956,7 @@ open_and_init_dwp_file (void) > error (_("Dwarf Error: DWP file CU version %s doesn't match" > " TU version %s [in DWP file %s]"), > pulongest (dwp_file->cus->version), > - pulongest (dwp_file->tus->version), dwp_name); > + pulongest (dwp_file->tus->version), dwp_name.c_str ()); > } > dwp_file->version = dwp_file->cus->version; > > diff --git a/gdb/python/python.c b/gdb/python/python.c > index 73fb3d0..a7aff53 100644 > --- a/gdb/python/python.c > +++ b/gdb/python/python.c > @@ -1550,7 +1550,7 @@ do_start_initialization () > /foo/bin/python > /foo/lib/pythonX.Y/... > This must be done before calling Py_Initialize. */ > - progname = concat (ldirname (python_libdir), SLASH_STRING, "bin", > + progname = concat (ldirname (python_libdir).c_str (), > SLASH_STRING, "bin", SLASH_STRING, "python", (char *) NULL); > #ifdef IS_PY3K > oldloc = xstrdup (setlocale (LC_ALL, NULL)); > diff --git a/gdb/utils.c b/gdb/utils.c > index 27021a1..39798cc 100644 > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -2943,20 +2943,19 @@ dummy_obstack_deallocate (void *object, void > *data) /* Simple, portable version of dirname that does not modify its > argument. */ > > -char * > +std::string > ldirname (const char *filename) > { > + std::string dirname; > const char *base = lbasename (filename); > - char *dirname; > > while (base > filename && IS_DIR_SEPARATOR (base[-1])) > --base; > > if (base == filename) > - return NULL; > + return dirname; > > - dirname = (char *) xmalloc (base - filename + 2); > - memcpy (dirname, filename, base - filename); > + dirname = std::string (filename, base - filename); > > /* On DOS based file systems, convert "d:foo" to "d:.", so that we > create "d:./bar" later instead of the (different) "d:/bar". */ > @@ -2964,7 +2963,6 @@ ldirname (const char *filename) > && !IS_DIR_SEPARATOR (filename[0])) > dirname[base++ - filename] = '.'; > > - dirname[base - filename] = '\0'; > return dirname; > } > > diff --git a/gdb/utils.h b/gdb/utils.h > index f138702..fb75f2e 100644 > --- a/gdb/utils.h > +++ b/gdb/utils.h > @@ -135,7 +135,7 @@ extern int gdb_filename_fnmatch (const char > *pattern, const char *string, extern void substitute_path_component > (char **stringp, const char *from, const char *to); > > -char *ldirname (const char *filename); > +std::string ldirname (const char *filename); > > extern int count_path_elements (const char *path); > > diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c > index 1e42b8d..a436418 100644 > --- a/gdb/xml-syscall.c > +++ b/gdb/xml-syscall.c > @@ -363,7 +363,6 @@ static struct syscalls_info * > xml_init_syscalls_info (const char *filename) > { > char *full_file; > - char *dirname; > struct syscalls_info *syscalls_info; > struct cleanup *back_to; > > @@ -373,12 +372,9 @@ xml_init_syscalls_info (const char *filename) > > back_to = make_cleanup (xfree, full_file); > > - dirname = ldirname (filename); > - if (dirname != NULL) > - make_cleanup (xfree, dirname); > - > syscalls_info = syscall_parse_xml (full_file, > - xml_fetch_content_from_file, > dirname); > + xml_fetch_content_from_file, > + (void *) ldirname > (filename).c_str ()); do_cleanups (back_to); > > return syscalls_info; > diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c > index 1677659..effe652 100644 > --- a/gdb/xml-tdesc.c > +++ b/gdb/xml-tdesc.c > @@ -694,7 +694,6 @@ file_read_description_xml (const char *filename) > struct target_desc *tdesc; > char *tdesc_str; > struct cleanup *back_to; > - char *dirname; > > tdesc_str = xml_fetch_content_from_file (filename, NULL); > if (tdesc_str == NULL) > @@ -705,11 +704,8 @@ file_read_description_xml (const char *filename) > > back_to = make_cleanup (xfree, tdesc_str); > > - dirname = ldirname (filename); > - if (dirname != NULL) > - make_cleanup (xfree, dirname); > - > - tdesc = tdesc_parse_xml (tdesc_str, xml_fetch_content_from_file, > dirname); > + tdesc = tdesc_parse_xml (tdesc_str, xml_fetch_content_from_file, > + (void *) ldirname (filename).c_str ()); > do_cleanups (back_to); > > return tdesc;