From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21614 invoked by alias); 19 Oct 2016 23:48:49 -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 21597 invoked by uid 89); 19 Oct 2016 23:48:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.5 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD,SPF_HELO_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy=dc, d.c, sk:discard, INPUT X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Oct 2016 23:48:46 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 69AC48553E; Wed, 19 Oct 2016 23:48:45 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9JNmhrU006449; Wed, 19 Oct 2016 19:48:44 -0400 Subject: Re: [PATCH v2 21/31] Use ui_file_as_string in gdb/compile/ To: Simon Marchi References: <1476839539-8374-1-git-send-email-palves@redhat.com> <1476839539-8374-22-git-send-email-palves@redhat.com> <8a267925d9af7f5b01c75a81da15b664@simark.ca> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <7a6e4bde-9ca2-4163-bdd7-0fd1f88a516b@redhat.com> Date: Wed, 19 Oct 2016 23:48:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <8a267925d9af7f5b01c75a81da15b664@simark.ca> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00594.txt.bz2 On 10/20/2016 12:08 AM, Simon Marchi wrote: > The doc for get_new_file_names should probably be updated: > > /* Compute the names of source and object files to use. The names are > allocated by malloc and should be freed by the caller. */ > >> static void >> -get_new_file_names (char **source_file, char **object_file) >> +get_new_file_names (std::string *source_file, std::string *object_file) >> { > > > Same for compile_to_object, both for the "freeing" part and the part > that talks about the return value. > > We should use $other_language instead of C++, to be able to do > > source_filep, object_filep = compile_to_object (...) > > :) Well, there's std::pair in C++03, and std::tuple in C++11. And C++17 has structured bindings: auto [source_file, object_file] = compile_to_object (); :-) But I think returning a real named struct is clearer than all that. See patch below showing the idea. WDYT? > > Btw, would it be good to use references for those strings? Maybe. I had kept it using pointers to make it clearer at the call site that these are output parameters. Maybe that's old-school C++ mentality? That moot with this version of the patch though, so I can dodge discussing that for now. :-) > >> error condition, error () is called. The caller is responsible for >> freeing both strings. */ >> >> -static char * >> +static void >> compile_to_object (struct command_line *cmd, const char *cmd_string, >> enum compile_i_scope_types scope, >> - char **source_filep) >> + std::string *source_filep, >> + std::string *object_filep) >> { > > ... > >> @@ -609,30 +606,27 @@ void >> eval_compile_command (struct command_line *cmd, const char *cmd_string, >> enum compile_i_scope_types scope, void *scope_data) >> { >> - char *object_file, *source_file; >> + std::string source_file, object_file; >> >> - object_file = compile_to_object (cmd, cmd_string, scope, >> &source_file); >> - if (object_file != NULL) >> + compile_to_object (cmd, cmd_string, scope, &source_file, >> &object_file); >> + if (!object_file.empty ()) > > Can this actually happen? Hmm, looks like not. This passes the gdb.compile/ tests. >From 80470a5ac4b3db444f2bc605a639ca6ef795f743 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 20 Oct 2016 00:28:26 +0100 Subject: [PATCH] Use ui_file_as_string in gdb/compile/ gdb/ChangeLog: yyyy-mm-yy Pedro Alves * c-lang.h (c_compute_program): Now returns std::string. * compile/compile-c-support.c (c_compute_program): Now returns a std::string. Use ui_file_as_string. * compile/compile.c (get_new_file_names): Change output parameters to std::string pointers. Use string_printf. (compile_to_object): Change output parameters to std::string pointers. Use ui_file_as_string. (eval_compile_command): Use std::string. * language.h (struct language_defn) : Now returns std::string. --- gdb/c-lang.h | 14 ++--- gdb/compile/compile-c-support.c | 6 +- gdb/compile/compile.c | 132 ++++++++++++++++++++++------------------ gdb/language.h | 14 ++--- 4 files changed, 89 insertions(+), 77 deletions(-) diff --git a/gdb/c-lang.h b/gdb/c-lang.h index 12be8bf..8378d4f 100644 --- a/gdb/c-lang.h +++ b/gdb/c-lang.h @@ -152,16 +152,16 @@ extern int c_textual_element_type (struct type *, char); extern struct compile_instance *c_get_compile_context (void); -/* This takes the user-supplied text and returns a newly malloc'd bit - of code to compile. +/* This takes the user-supplied text and returns a new bit of code to + compile. This is used as the la_compute_program language method; see that for a description of the arguments. */ -extern char *c_compute_program (struct compile_instance *inst, - const char *input, - struct gdbarch *gdbarch, - const struct block *expr_block, - CORE_ADDR expr_pc); +extern std::string c_compute_program (struct compile_instance *inst, + const char *input, + struct gdbarch *gdbarch, + const struct block *expr_block, + CORE_ADDR expr_pc); #endif /* !defined (C_LANG_H) */ diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c index c42daba..641b0fd 100644 --- a/gdb/compile/compile-c-support.c +++ b/gdb/compile/compile-c-support.c @@ -326,7 +326,7 @@ generate_register_struct (struct ui_file *stream, struct gdbarch *gdbarch, to the inferior when the expression was created, and EXPR_PC indicates the value of $PC. */ -char * +std::string c_compute_program (struct compile_instance *inst, const char *input, struct gdbarch *gdbarch, @@ -334,7 +334,7 @@ c_compute_program (struct compile_instance *inst, CORE_ADDR expr_pc) { struct ui_file *buf, *var_stream = NULL; - char *code; + std::string code; struct cleanup *cleanup; struct compile_c_instance *context = (struct compile_c_instance *) inst; @@ -435,7 +435,7 @@ c_compute_program (struct compile_instance *inst, fputs_unfiltered ("}\n", buf); add_code_footer (inst->scope, buf); - code = ui_file_xstrdup (buf, NULL); + code = ui_file_as_string (buf); do_cleanups (cleanup); return code; } diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c index 0c4a738..08dfdc2 100644 --- a/gdb/compile/compile.c +++ b/gdb/compile/compile.c @@ -255,18 +255,40 @@ get_compile_file_tempdir (void) return tempdir_name; } -/* Compute the names of source and object files to use. The names are - allocated by malloc and should be freed by the caller. */ - -static void -get_new_file_names (char **source_file, char **object_file) +class compile_file_names +{ +public: + compile_file_names(std::string source_file, std::string object_file) + : m_source_file (source_file), m_object_file (object_file) + {} + + const char *source_file () + { + return m_source_file.c_str (); + } + + const char *object_file () + { + return m_object_file.c_str (); + } + +private: + std::string m_source_file; + std::string m_object_file; +}; + +/* Compute the names of source and object files to use. */ + +static compile_file_names +get_new_file_names () { static int seq; const char *dir = get_compile_file_tempdir (); ++seq; - *source_file = xstrprintf ("%s%sout%d.c", dir, SLASH_STRING, seq); - *object_file = xstrprintf ("%s%sout%d.o", dir, SLASH_STRING, seq); + + return compile_file_names (string_printf ("%s%sout%d.c", dir, SLASH_STRING, seq), + string_printf ("%s%sout%d.o", dir, SLASH_STRING, seq)); } /* Get the block and PC at which to evaluate an expression. */ @@ -456,18 +478,13 @@ print_callback (void *ignore, const char *message) } /* Process the compilation request. On success it returns the object - file name and *SOURCE_FILEP is set to source file name. On an - error condition, error () is called. The caller is responsible for - freeing both strings. */ + and source file names. On an error condition, error () is + called. */ -static char * +static compile_file_names compile_to_object (struct command_line *cmd, const char *cmd_string, - enum compile_i_scope_types scope, - char **source_filep) + enum compile_i_scope_types scope) { - char *code; - const char *input; - char *source_file, *object_file; struct compile_instance *compiler; struct cleanup *cleanup, *inner_cleanup; const struct block *expr_block; @@ -503,11 +520,14 @@ compile_to_object (struct command_line *cmd, const char *cmd_string, /* From the provided expression, build a scope to pass to the compiler. */ + + std::string input_buf; + const char *input; + if (cmd != NULL) { struct ui_file *stream = mem_fileopen (); struct command_line *iter; - char *stream_buf; make_cleanup_ui_file_delete (stream); for (iter = cmd->body_list[0]; iter; iter = iter->next) @@ -516,20 +536,19 @@ compile_to_object (struct command_line *cmd, const char *cmd_string, fputs_unfiltered ("\n", stream); } - stream_buf = ui_file_xstrdup (stream, NULL); - make_cleanup (xfree, stream_buf); - input = stream_buf; + input_buf = ui_file_as_string (stream); + input = input_buf.c_str (); } else if (cmd_string != NULL) input = cmd_string; else error (_("Neither a simple expression, or a multi-line specified.")); - code = current_language->la_compute_program (compiler, input, gdbarch, - expr_block, expr_pc); - make_cleanup (xfree, code); + std::string code + = current_language->la_compute_program (compiler, input, gdbarch, + expr_block, expr_pc); if (compile_debug) - fprintf_unfiltered (gdb_stdlog, "debug output:\n\n%s", code); + fprintf_unfiltered (gdb_stdlog, "debug output:\n\n%s", code.c_str ()); os_rx = osabi_triplet_regexp (gdbarch_osabi (gdbarch)); arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); @@ -560,37 +579,36 @@ compile_to_object (struct command_line *cmd, const char *cmd_string, argi, argv[argi]); } - get_new_file_names (&source_file, &object_file); - inner_cleanup = make_cleanup (xfree, source_file); - make_cleanup (xfree, object_file); + compile_file_names fnames = get_new_file_names (); - src = gdb_fopen_cloexec (source_file, "w"); + src = gdb_fopen_cloexec (fnames.source_file (), "w"); if (src == NULL) perror_with_name (_("Could not open source file for writing")); - make_cleanup (cleanup_unlink_file, source_file); - if (fputs (code, src) == EOF) + inner_cleanup = make_cleanup (cleanup_unlink_file, + (void *) fnames.source_file ()); + if (fputs (code.c_str (), src) == EOF) perror_with_name (_("Could not write to source file")); fclose (src); if (compile_debug) fprintf_unfiltered (gdb_stdlog, "source file produced: %s\n\n", - source_file); + fnames.source_file ()); /* Call the compiler and start the compilation process. */ - compiler->fe->ops->set_source_file (compiler->fe, source_file); + compiler->fe->ops->set_source_file (compiler->fe, fnames.source_file ()); - if (!compiler->fe->ops->compile (compiler->fe, object_file, + if (!compiler->fe->ops->compile (compiler->fe, fnames.object_file (), compile_debug)) error (_("Compilation failed.")); if (compile_debug) fprintf_unfiltered (gdb_stdlog, "object file produced: %s\n\n", - object_file); + fnames.object_file ()); discard_cleanups (inner_cleanup); do_cleanups (cleanup); - *source_filep = source_file; - return object_file; + + return fnames; } /* The "compile" prefix command. */ @@ -609,32 +627,26 @@ void eval_compile_command (struct command_line *cmd, const char *cmd_string, enum compile_i_scope_types scope, void *scope_data) { - char *object_file, *source_file; - - object_file = compile_to_object (cmd, cmd_string, scope, &source_file); - if (object_file != NULL) + struct cleanup *cleanup_unlink; + struct compile_module *compile_module; + + compile_file_names fnames = compile_to_object (cmd, cmd_string, scope); + + cleanup_unlink = make_cleanup (cleanup_unlink_file, + (void *) fnames.object_file ()); + make_cleanup (cleanup_unlink_file, (void *) fnames.source_file ()); + compile_module = compile_object_load (fnames.object_file (), + fnames.source_file (), + scope, scope_data); + if (compile_module == NULL) { - struct cleanup *cleanup_xfree, *cleanup_unlink; - struct compile_module *compile_module; - - cleanup_xfree = make_cleanup (xfree, object_file); - make_cleanup (xfree, source_file); - cleanup_unlink = make_cleanup (cleanup_unlink_file, object_file); - make_cleanup (cleanup_unlink_file, source_file); - compile_module = compile_object_load (object_file, source_file, - scope, scope_data); - if (compile_module == NULL) - { - gdb_assert (scope == COMPILE_I_PRINT_ADDRESS_SCOPE); - do_cleanups (cleanup_xfree); - eval_compile_command (cmd, cmd_string, - COMPILE_I_PRINT_VALUE_SCOPE, scope_data); - return; - } - discard_cleanups (cleanup_unlink); - do_cleanups (cleanup_xfree); - compile_object_run (compile_module); + gdb_assert (scope == COMPILE_I_PRINT_ADDRESS_SCOPE); + eval_compile_command (cmd, cmd_string, + COMPILE_I_PRINT_VALUE_SCOPE, scope_data); + return; } + discard_cleanups (cleanup_unlink); + compile_object_run (compile_module); } /* See compile/compile-internal.h. */ diff --git a/gdb/language.h b/gdb/language.h index d6f932e..758f265 100644 --- a/gdb/language.h +++ b/gdb/language.h @@ -406,8 +406,8 @@ struct language_defn If 'la_get_gcc_context' is not defined, then this method is ignored. - This takes the user-supplied text and returns a newly malloc'd - bit of code to compile. The caller owns the result. + This takes the user-supplied text and returns a new bit of code + to compile. INST is the compiler instance being used. INPUT is the user's input text. @@ -416,11 +416,11 @@ struct language_defn parsed. EXPR_PC is the PC at which the expression is being parsed. */ - char *(*la_compute_program) (struct compile_instance *inst, - const char *input, - struct gdbarch *gdbarch, - const struct block *expr_block, - CORE_ADDR expr_pc); + std::string (*la_compute_program) (struct compile_instance *inst, + const char *input, + struct gdbarch *gdbarch, + const struct block *expr_block, + CORE_ADDR expr_pc); /* Add fields above this point, so the magic number is always last. */ /* Magic number for compat checking. */ -- 2.5.5