* [RFA] Remove a cleanup from parse_expression_for_completion @ 2018-02-16 23:02 Tom Tromey 2018-02-16 23:27 ` Simon Marchi 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2018-02-16 23:02 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This removes a cleanup from parse_expression_for_completion, by changing various expression-completion functions to use std::string rather than explicit malloc+free. Regression tested by the buildbot. gdb/ChangeLog 2018-02-16 Tom Tromey <tom@tromey.com> * value.h: (extract_field_op): Update. * eval.c (extract_field_op): Return a const char *. * expression.h (parse_expression_for_completion): Update. * completer.c (complete_expression): Update. (add_struct_fields): Make fieldname const. * parse.c (expout_completion_name): Now a std::string. (mark_completion_tag, parse_exp_in_context_1): Update. (parse_expression_for_completion): Change "name" to std::string*. --- gdb/ChangeLog | 11 +++++++++++ gdb/completer.c | 22 ++++++++-------------- gdb/eval.c | 2 +- gdb/expression.h | 3 ++- gdb/parse.c | 20 +++++++------------- gdb/value.h | 2 +- 6 files changed, 30 insertions(+), 30 deletions(-) diff --git a/gdb/completer.c b/gdb/completer.c index a71bd368ff..c0d5778867 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -962,7 +962,7 @@ location_completer_handle_brkchars (struct cmd_list_element *ignore, static void add_struct_fields (struct type *type, completion_list &output, - char *fieldname, int namelen) + const char *fieldname, int namelen) { int i; int computed_type_name = 0; @@ -1016,12 +1016,11 @@ complete_expression (completion_tracker &tracker, const char *text, const char *word) { struct type *type = NULL; - char *fieldname; + std::string fieldname; enum type_code code = TYPE_CODE_UNDEF; /* Perform a tentative parse of the expression, to see whether a field completion is required. */ - fieldname = NULL; TRY { type = parse_expression_for_completion (text, &fieldname, &code); @@ -1032,7 +1031,7 @@ complete_expression (completion_tracker &tracker, } END_CATCH - if (fieldname && type) + if (!fieldname.empty () && type) { for (;;) { @@ -1045,25 +1044,20 @@ complete_expression (completion_tracker &tracker, if (TYPE_CODE (type) == TYPE_CODE_UNION || TYPE_CODE (type) == TYPE_CODE_STRUCT) { - int flen = strlen (fieldname); completion_list result; - add_struct_fields (type, result, fieldname, flen); - xfree (fieldname); + add_struct_fields (type, result, fieldname.c_str (), + fieldname.size ()); tracker.add_completions (std::move (result)); return; } } - else if (fieldname && code != TYPE_CODE_UNDEF) + else if (!fieldname.empty () && code != TYPE_CODE_UNDEF) { - struct cleanup *cleanup = make_cleanup (xfree, fieldname); - - collect_symbol_completion_matches_type (tracker, fieldname, fieldname, - code); - do_cleanups (cleanup); + collect_symbol_completion_matches_type (tracker, fieldname.c_str (), + fieldname.c_str (), code); return; } - xfree (fieldname); complete_files_symbols (tracker, text, word); } diff --git a/gdb/eval.c b/gdb/eval.c index 6f74c41b9f..4899011a58 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -272,7 +272,7 @@ fetch_subexp_value (struct expression *exp, int *pc, struct value **valp, subexpression of the left-hand-side of the dereference. This is used when completing field names. */ -char * +const char * extract_field_op (struct expression *exp, int *subexp) { int tem; diff --git a/gdb/expression.h b/gdb/expression.h index 030f2f08e7..f9b5b1e820 100644 --- a/gdb/expression.h +++ b/gdb/expression.h @@ -101,7 +101,8 @@ extern expression_up parse_expression (const char *); extern expression_up parse_expression_with_language (const char *string, enum language lang); -extern struct type *parse_expression_for_completion (const char *, char **, +extern struct type *parse_expression_for_completion (const char *, + std::string *, enum type_code *); extern expression_up parse_exp_1 (const char **, CORE_ADDR pc, diff --git a/gdb/parse.c b/gdb/parse.c index e3f1306a17..1d1ede4f56 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -88,7 +88,7 @@ static int expout_last_struct = -1; static enum type_code expout_tag_completion_type = TYPE_CODE_UNDEF; /* The token for tagged type name completion. */ -static char *expout_completion_name; +static std::string expout_completion_name; \f static unsigned int expressiondebug = 0; @@ -569,15 +569,13 @@ mark_completion_tag (enum type_code tag, const char *ptr, int length) { gdb_assert (parse_completion && expout_tag_completion_type == TYPE_CODE_UNDEF - && expout_completion_name == NULL + && expout_completion_name.empty () && expout_last_struct == -1); gdb_assert (tag == TYPE_CODE_UNION || tag == TYPE_CODE_STRUCT || tag == TYPE_CODE_ENUM); expout_tag_completion_type = tag; - expout_completion_name = (char *) xmalloc (length + 1); - memcpy (expout_completion_name, ptr, length); - expout_completion_name[length] = '\0'; + expout_completion_name = std::string (ptr, length); } \f @@ -1137,8 +1135,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, type_stack.depth = 0; expout_last_struct = -1; expout_tag_completion_type = TYPE_CODE_UNDEF; - xfree (expout_completion_name); - expout_completion_name = NULL; + expout_completion_name = ""; comma_terminates = comma; @@ -1281,7 +1278,7 @@ parse_expression_with_language (const char *string, enum language lang) *NAME must be freed by the caller. */ struct type * -parse_expression_for_completion (const char *string, char **name, +parse_expression_for_completion (const char *string, std::string *name, enum type_code *code) { expression_up exp; @@ -1306,8 +1303,7 @@ parse_expression_for_completion (const char *string, char **name, if (expout_tag_completion_type != TYPE_CODE_UNDEF) { *code = expout_tag_completion_type; - *name = expout_completion_name; - expout_completion_name = NULL; + *name = std::move (expout_completion_name); return NULL; } @@ -1315,14 +1311,12 @@ parse_expression_for_completion (const char *string, char **name, return NULL; *name = extract_field_op (exp.get (), &subexp); - if (!*name) + if (name->empty ()) return NULL; /* This might throw an exception. If so, we want to let it propagate. */ val = evaluate_subexpression_type (exp.get (), subexp); - /* (*NAME) is a part of the EXP memory block freed below. */ - *name = xstrdup (*name); return value_type (val); } diff --git a/gdb/value.h b/gdb/value.h index e0ea22d4e5..5676d245b9 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -889,7 +889,7 @@ extern void fetch_subexp_value (struct expression *exp, int *pc, struct value **val_chain, int preserve_errors); -extern char *extract_field_op (struct expression *exp, int *subexp); +extern const char *extract_field_op (struct expression *exp, int *subexp); extern struct value *evaluate_subexp_with_coercion (struct expression *, int *, enum noside); -- 2.13.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Remove a cleanup from parse_expression_for_completion 2018-02-16 23:02 [RFA] Remove a cleanup from parse_expression_for_completion Tom Tromey @ 2018-02-16 23:27 ` Simon Marchi 2018-02-20 4:39 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Simon Marchi @ 2018-02-16 23:27 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 2018-02-16 18:02, Tom Tromey wrote: > This removes a cleanup from parse_expression_for_completion, by > changing various expression-completion functions to use std::string > rather than explicit malloc+free. That looks good to me. You can go a bit further if you want and make add_struct_fields take a const std::string reference as well. Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Remove a cleanup from parse_expression_for_completion 2018-02-16 23:27 ` Simon Marchi @ 2018-02-20 4:39 ` Tom Tromey 2018-02-20 5:02 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2018-02-20 4:39 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: Simon> On 2018-02-16 18:02, Tom Tromey wrote: >> This removes a cleanup from parse_expression_for_completion, by >> changing various expression-completion functions to use std::string >> rather than explicit malloc+free. Simon> That looks good to me. You can go a bit further if you want and make Simon> add_struct_fields take a const std::string reference as well. Here's an updated patch. Let me know what you think. Tom commit a922232c1fc11a5f932140d8dfa8fcea963cecad Author: Tom Tromey <tom@tromey.com> Date: Thu Feb 15 22:41:03 2018 -0700 Remove a cleanup from parse_expression_for_completion This removes a cleanup from parse_expression_for_completion, by changing various expression-completion functions to use std::string rather than explicit malloc+free. Regression tested by the buildbot. gdb/ChangeLog 2018-02-19 Tom Tromey <tom@tromey.com> * value.h: (extract_field_op): Update. * eval.c (extract_field_op): Return a const char *. * expression.h (parse_expression_for_completion): Update. * completer.c (complete_expression): Update. (add_struct_fields): Make fieldname std::string. Remove namelen. * parse.c (expout_completion_name): Now a std::string. (mark_completion_tag, parse_exp_in_context_1): Update. (parse_expression_for_completion): Change "name" to std::string*. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index cd8ef6653d..ac2840d0cb 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2018-02-19 Tom Tromey <tom@tromey.com> + + * value.h: (extract_field_op): Update. + * eval.c (extract_field_op): Return a const char *. + * expression.h (parse_expression_for_completion): Update. + * completer.c (complete_expression): Update. + (add_struct_fields): Make fieldname std::string. Remove namelen. + * parse.c (expout_completion_name): Now a std::string. + (mark_completion_tag, parse_exp_in_context_1): Update. + (parse_expression_for_completion): Change "name" to std::string*. + 2018-02-19 Alan Hayward <alan.hayward@arm.com> * Makefile.in: (COMMON_SFILES): Add common/*.c files. diff --git a/gdb/completer.c b/gdb/completer.c index a71bd368ff..6d6d52a109 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -962,7 +962,7 @@ location_completer_handle_brkchars (struct cmd_list_element *ignore, static void add_struct_fields (struct type *type, completion_list &output, - char *fieldname, int namelen) + const std::string &fieldname) { int i; int computed_type_name = 0; @@ -973,20 +973,20 @@ add_struct_fields (struct type *type, completion_list &output, { if (i < TYPE_N_BASECLASSES (type)) add_struct_fields (TYPE_BASECLASS (type, i), - output, fieldname, namelen); + output, fieldname); else if (TYPE_FIELD_NAME (type, i)) { if (TYPE_FIELD_NAME (type, i)[0] != '\0') { if (! strncmp (TYPE_FIELD_NAME (type, i), - fieldname, namelen)) + fieldname.c_str (), fieldname.size ())) output.emplace_back (xstrdup (TYPE_FIELD_NAME (type, i))); } else if (TYPE_CODE (TYPE_FIELD_TYPE (type, i)) == TYPE_CODE_UNION) { /* Recurse into anonymous unions. */ add_struct_fields (TYPE_FIELD_TYPE (type, i), - output, fieldname, namelen); + output, fieldname); } } } @@ -995,7 +995,7 @@ add_struct_fields (struct type *type, completion_list &output, { const char *name = TYPE_FN_FIELDLIST_NAME (type, i); - if (name && ! strncmp (name, fieldname, namelen)) + if (name && ! strncmp (name, fieldname.c_str (), fieldname.size ())) { if (!computed_type_name) { @@ -1016,12 +1016,11 @@ complete_expression (completion_tracker &tracker, const char *text, const char *word) { struct type *type = NULL; - char *fieldname; + std::string fieldname; enum type_code code = TYPE_CODE_UNDEF; /* Perform a tentative parse of the expression, to see whether a field completion is required. */ - fieldname = NULL; TRY { type = parse_expression_for_completion (text, &fieldname, &code); @@ -1032,7 +1031,7 @@ complete_expression (completion_tracker &tracker, } END_CATCH - if (fieldname && type) + if (!fieldname.empty () && type) { for (;;) { @@ -1045,25 +1044,19 @@ complete_expression (completion_tracker &tracker, if (TYPE_CODE (type) == TYPE_CODE_UNION || TYPE_CODE (type) == TYPE_CODE_STRUCT) { - int flen = strlen (fieldname); completion_list result; - add_struct_fields (type, result, fieldname, flen); - xfree (fieldname); + add_struct_fields (type, result, fieldname); tracker.add_completions (std::move (result)); return; } } - else if (fieldname && code != TYPE_CODE_UNDEF) + else if (!fieldname.empty () && code != TYPE_CODE_UNDEF) { - struct cleanup *cleanup = make_cleanup (xfree, fieldname); - - collect_symbol_completion_matches_type (tracker, fieldname, fieldname, - code); - do_cleanups (cleanup); + collect_symbol_completion_matches_type (tracker, fieldname.c_str (), + fieldname.c_str (), code); return; } - xfree (fieldname); complete_files_symbols (tracker, text, word); } diff --git a/gdb/eval.c b/gdb/eval.c index 6f74c41b9f..4899011a58 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -272,7 +272,7 @@ fetch_subexp_value (struct expression *exp, int *pc, struct value **valp, subexpression of the left-hand-side of the dereference. This is used when completing field names. */ -char * +const char * extract_field_op (struct expression *exp, int *subexp) { int tem; diff --git a/gdb/expression.h b/gdb/expression.h index 030f2f08e7..f9b5b1e820 100644 --- a/gdb/expression.h +++ b/gdb/expression.h @@ -101,7 +101,8 @@ extern expression_up parse_expression (const char *); extern expression_up parse_expression_with_language (const char *string, enum language lang); -extern struct type *parse_expression_for_completion (const char *, char **, +extern struct type *parse_expression_for_completion (const char *, + std::string *, enum type_code *); extern expression_up parse_exp_1 (const char **, CORE_ADDR pc, diff --git a/gdb/parse.c b/gdb/parse.c index e3f1306a17..4c09ebec7d 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -88,7 +88,7 @@ static int expout_last_struct = -1; static enum type_code expout_tag_completion_type = TYPE_CODE_UNDEF; /* The token for tagged type name completion. */ -static char *expout_completion_name; +static std::string expout_completion_name; \f static unsigned int expressiondebug = 0; @@ -569,15 +569,13 @@ mark_completion_tag (enum type_code tag, const char *ptr, int length) { gdb_assert (parse_completion && expout_tag_completion_type == TYPE_CODE_UNDEF - && expout_completion_name == NULL + && expout_completion_name.empty () && expout_last_struct == -1); gdb_assert (tag == TYPE_CODE_UNION || tag == TYPE_CODE_STRUCT || tag == TYPE_CODE_ENUM); expout_tag_completion_type = tag; - expout_completion_name = (char *) xmalloc (length + 1); - memcpy (expout_completion_name, ptr, length); - expout_completion_name[length] = '\0'; + expout_completion_name = std::string (ptr, length); } \f @@ -1137,8 +1135,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, type_stack.depth = 0; expout_last_struct = -1; expout_tag_completion_type = TYPE_CODE_UNDEF; - xfree (expout_completion_name); - expout_completion_name = NULL; + expout_completion_name.clear (); comma_terminates = comma; @@ -1277,11 +1274,10 @@ parse_expression_with_language (const char *string, enum language lang) reference; furthermore, if the parsing ends in the field name, return the field name in *NAME. If the parsing ends in the middle of a field reference, but the reference is somehow invalid, throw - an exception. In all other cases, return NULL. Returned non-NULL - *NAME must be freed by the caller. */ + an exception. In all other cases, return NULL. */ struct type * -parse_expression_for_completion (const char *string, char **name, +parse_expression_for_completion (const char *string, std::string *name, enum type_code *code) { expression_up exp; @@ -1306,23 +1302,21 @@ parse_expression_for_completion (const char *string, char **name, if (expout_tag_completion_type != TYPE_CODE_UNDEF) { *code = expout_tag_completion_type; - *name = expout_completion_name; - expout_completion_name = NULL; + *name = std::move (expout_completion_name); return NULL; } if (expout_last_struct == -1) return NULL; - *name = extract_field_op (exp.get (), &subexp); - if (!*name) + const char *fieldname = extract_field_op (exp.get (), &subexp); + if (fieldname == NULL) return NULL; + *name = fieldname; /* This might throw an exception. If so, we want to let it propagate. */ val = evaluate_subexpression_type (exp.get (), subexp); - /* (*NAME) is a part of the EXP memory block freed below. */ - *name = xstrdup (*name); return value_type (val); } diff --git a/gdb/value.h b/gdb/value.h index e0ea22d4e5..5676d245b9 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -889,7 +889,7 @@ extern void fetch_subexp_value (struct expression *exp, int *pc, struct value **val_chain, int preserve_errors); -extern char *extract_field_op (struct expression *exp, int *subexp); +extern const char *extract_field_op (struct expression *exp, int *subexp); extern struct value *evaluate_subexp_with_coercion (struct expression *, int *, enum noside); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Remove a cleanup from parse_expression_for_completion 2018-02-20 4:39 ` Tom Tromey @ 2018-02-20 5:02 ` Tom Tromey 2018-02-20 20:23 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2018-02-20 5:02 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: Tom> Here's an updated patch. Let me know what you think. Actually, this patch is wrong, and I didn't realize it until this revision. There was a failure in the buildbot report that I didn't notice in the noise :( There is a case in here where the NULL/empty string distinction matters. So I think we can't use std::string but must instead use unique_xmalloc_ptr. I'll update the patch. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Remove a cleanup from parse_expression_for_completion 2018-02-20 5:02 ` Tom Tromey @ 2018-02-20 20:23 ` Tom Tromey 2018-02-21 3:46 ` Simon Marchi 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2018-02-20 20:23 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: Tom> There is a case in here where the NULL/empty string distinction matters. Tom> So I think we can't use std::string but must instead use Tom> unique_xmalloc_ptr. I'll update the patch. Here's the new version. Re-tested on the buildbot & I looked more carefully at the results this time; also I tried all the completion-related tests locally as well. Tom commit 3de98da217150149d26dadee2c0ca9825463adda Author: Tom Tromey <tom@tromey.com> Date: Thu Feb 15 22:41:03 2018 -0700 Remove a cleanup from parse_expression_for_completion This removes a cleanup from parse_expression_for_completion, by changing various expression-completion functions to use gdb::unique_xmalloc_ptry rather than explicit malloc+free. Regression tested by the buildbot. gdb/ChangeLog 2018-02-19 Tom Tromey <tom@tromey.com> * value.h: (extract_field_op): Update. * eval.c (extract_field_op): Return a const char *. * expression.h (parse_expression_for_completion): Update. * completer.c (complete_expression): Update. (add_struct_fields): Make fieldname const. * parse.c (expout_completion_name): Now a unique_xmalloc_ptr. (mark_completion_tag, parse_exp_in_context_1): Update. (parse_expression_for_completion): Change "name" to unique_xmalloc_ptr*. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index cd8ef6653d..ca660163fa 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2018-02-19 Tom Tromey <tom@tromey.com> + + * value.h: (extract_field_op): Update. + * eval.c (extract_field_op): Return a const char *. + * expression.h (parse_expression_for_completion): Update. + * completer.c (complete_expression): Update. + (add_struct_fields): Make fieldname const. + * parse.c (expout_completion_name): Now a unique_xmalloc_ptr. + (mark_completion_tag, parse_exp_in_context_1): Update. + (parse_expression_for_completion): Change "name" to + unique_xmalloc_ptr*. + 2018-02-19 Alan Hayward <alan.hayward@arm.com> * Makefile.in: (COMMON_SFILES): Add common/*.c files. diff --git a/gdb/completer.c b/gdb/completer.c index a71bd368ff..4de1bcff32 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -962,7 +962,7 @@ location_completer_handle_brkchars (struct cmd_list_element *ignore, static void add_struct_fields (struct type *type, completion_list &output, - char *fieldname, int namelen) + const char *fieldname, int namelen) { int i; int computed_type_name = 0; @@ -1016,12 +1016,11 @@ complete_expression (completion_tracker &tracker, const char *text, const char *word) { struct type *type = NULL; - char *fieldname; + gdb::unique_xmalloc_ptr<char> fieldname; enum type_code code = TYPE_CODE_UNDEF; /* Perform a tentative parse of the expression, to see whether a field completion is required. */ - fieldname = NULL; TRY { type = parse_expression_for_completion (text, &fieldname, &code); @@ -1032,7 +1031,7 @@ complete_expression (completion_tracker &tracker, } END_CATCH - if (fieldname && type) + if (fieldname != nullptr && type) { for (;;) { @@ -1045,25 +1044,20 @@ complete_expression (completion_tracker &tracker, if (TYPE_CODE (type) == TYPE_CODE_UNION || TYPE_CODE (type) == TYPE_CODE_STRUCT) { - int flen = strlen (fieldname); completion_list result; - add_struct_fields (type, result, fieldname, flen); - xfree (fieldname); + add_struct_fields (type, result, fieldname.get (), + strlen (fieldname.get ())); tracker.add_completions (std::move (result)); return; } } - else if (fieldname && code != TYPE_CODE_UNDEF) + else if (fieldname != nullptr && code != TYPE_CODE_UNDEF) { - struct cleanup *cleanup = make_cleanup (xfree, fieldname); - - collect_symbol_completion_matches_type (tracker, fieldname, fieldname, - code); - do_cleanups (cleanup); + collect_symbol_completion_matches_type (tracker, fieldname.get (), + fieldname.get (), code); return; } - xfree (fieldname); complete_files_symbols (tracker, text, word); } diff --git a/gdb/eval.c b/gdb/eval.c index 6f74c41b9f..4899011a58 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -272,7 +272,7 @@ fetch_subexp_value (struct expression *exp, int *pc, struct value **valp, subexpression of the left-hand-side of the dereference. This is used when completing field names. */ -char * +const char * extract_field_op (struct expression *exp, int *subexp) { int tem; diff --git a/gdb/expression.h b/gdb/expression.h index 030f2f08e7..59a0898805 100644 --- a/gdb/expression.h +++ b/gdb/expression.h @@ -101,7 +101,8 @@ extern expression_up parse_expression (const char *); extern expression_up parse_expression_with_language (const char *string, enum language lang); -extern struct type *parse_expression_for_completion (const char *, char **, +extern struct type *parse_expression_for_completion (const char *, + gdb::unique_xmalloc_ptr<char> *, enum type_code *); extern expression_up parse_exp_1 (const char **, CORE_ADDR pc, diff --git a/gdb/parse.c b/gdb/parse.c index e3f1306a17..3abb9d4219 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -88,7 +88,7 @@ static int expout_last_struct = -1; static enum type_code expout_tag_completion_type = TYPE_CODE_UNDEF; /* The token for tagged type name completion. */ -static char *expout_completion_name; +static gdb::unique_xmalloc_ptr<char> expout_completion_name; \f static unsigned int expressiondebug = 0; @@ -575,9 +575,7 @@ mark_completion_tag (enum type_code tag, const char *ptr, int length) || tag == TYPE_CODE_STRUCT || tag == TYPE_CODE_ENUM); expout_tag_completion_type = tag; - expout_completion_name = (char *) xmalloc (length + 1); - memcpy (expout_completion_name, ptr, length); - expout_completion_name[length] = '\0'; + expout_completion_name.reset (xstrndup (ptr, length)); } \f @@ -1137,8 +1135,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, type_stack.depth = 0; expout_last_struct = -1; expout_tag_completion_type = TYPE_CODE_UNDEF; - xfree (expout_completion_name); - expout_completion_name = NULL; + expout_completion_name.reset (); comma_terminates = comma; @@ -1277,11 +1274,11 @@ parse_expression_with_language (const char *string, enum language lang) reference; furthermore, if the parsing ends in the field name, return the field name in *NAME. If the parsing ends in the middle of a field reference, but the reference is somehow invalid, throw - an exception. In all other cases, return NULL. Returned non-NULL - *NAME must be freed by the caller. */ + an exception. In all other cases, return NULL. */ struct type * -parse_expression_for_completion (const char *string, char **name, +parse_expression_for_completion (const char *string, + gdb::unique_xmalloc_ptr<char> *name, enum type_code *code) { expression_up exp; @@ -1306,23 +1303,24 @@ parse_expression_for_completion (const char *string, char **name, if (expout_tag_completion_type != TYPE_CODE_UNDEF) { *code = expout_tag_completion_type; - *name = expout_completion_name; - expout_completion_name = NULL; + *name = std::move (expout_completion_name); return NULL; } if (expout_last_struct == -1) return NULL; - *name = extract_field_op (exp.get (), &subexp); - if (!*name) - return NULL; + const char *fieldname = extract_field_op (exp.get (), &subexp); + if (fieldname == NULL) + { + name->reset (); + return NULL; + } + name->reset (xstrdup (fieldname)); /* This might throw an exception. If so, we want to let it propagate. */ val = evaluate_subexpression_type (exp.get (), subexp); - /* (*NAME) is a part of the EXP memory block freed below. */ - *name = xstrdup (*name); return value_type (val); } diff --git a/gdb/value.h b/gdb/value.h index e0ea22d4e5..5676d245b9 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -889,7 +889,7 @@ extern void fetch_subexp_value (struct expression *exp, int *pc, struct value **val_chain, int preserve_errors); -extern char *extract_field_op (struct expression *exp, int *subexp); +extern const char *extract_field_op (struct expression *exp, int *subexp); extern struct value *evaluate_subexp_with_coercion (struct expression *, int *, enum noside); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Remove a cleanup from parse_expression_for_completion 2018-02-20 20:23 ` Tom Tromey @ 2018-02-21 3:46 ` Simon Marchi 0 siblings, 0 replies; 6+ messages in thread From: Simon Marchi @ 2018-02-21 3:46 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 2018-02-20 03:23 PM, Tom Tromey wrote: >>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: > > Tom> There is a case in here where the NULL/empty string distinction matters. > Tom> So I think we can't use std::string but must instead use > Tom> unique_xmalloc_ptr. I'll update the patch. Woops! > diff --git a/gdb/expression.h b/gdb/expression.h > index 030f2f08e7..59a0898805 100644 > --- a/gdb/expression.h > +++ b/gdb/expression.h > @@ -101,7 +101,8 @@ extern expression_up parse_expression (const char *); > extern expression_up parse_expression_with_language (const char *string, > enum language lang); > > -extern struct type *parse_expression_for_completion (const char *, char **, > +extern struct type *parse_expression_for_completion (const char *, > + gdb::unique_xmalloc_ptr<char> *, This line is now too long. Otherwise it looks good to me. Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-21 3:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-16 23:02 [RFA] Remove a cleanup from parse_expression_for_completion Tom Tromey 2018-02-16 23:27 ` Simon Marchi 2018-02-20 4:39 ` Tom Tromey 2018-02-20 5:02 ` Tom Tromey 2018-02-20 20:23 ` Tom Tromey 2018-02-21 3:46 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).