[PATCH v2 21/31] Use ui_file_as_string in gdb/compile/
Pedro Alves
palves@redhat.com
Wed Oct 19 23:48:00 GMT 2016
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 <palves@redhat.com>
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 <palves@redhat.com>
* 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) <la_compute_program>: 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
More information about the Gdb-patches
mailing list