Factor out rbreak_command with concat
Commit Message
The function behaviour should be unchanged.
len was always 0 , so I removed it.
xrealloc was always called on NULL string, so equivalent to xmalloc,
which concat uses.
---
gdb/ChangeLog | 4 ++++
gdb/symtab.c | 35 +++++++++--------------------------
2 files changed, 13 insertions(+), 26 deletions(-)
Comments
Ciro Santilli <ciro.santilli@gmail.com> writes:
> The function behaviour should be unchanged.
>
> len was always 0 , so I removed it.
>
> xrealloc was always called on NULL string, so equivalent to xmalloc,
> which concat uses.
>
> ---
> gdb/ChangeLog | 4 ++++
> gdb/symtab.c | 35 +++++++++--------------------------
> 2 files changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 407dfb4..3d8c416 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +4015-07-20 Ciro Santilli <ciro.santilli@gmail.com>
> +
> + * symtab.c (rbreak_command): Factor with concat.
> +
> 2015-07-15 Markus Metzger <markus.t.metzger@intel.com>
> Pedro Alves <palves@redhat.com>
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index decc5a9..015cb56 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4889,7 +4889,6 @@ rbreak_command (char *regexp, int from_tty)
> struct symbol_search *p;
> struct cleanup *old_chain;
> char *string = NULL;
> - int len = 0;
> const char **files = NULL;
> const char *file_name;
> int nfiles = 0;
> @@ -4928,20 +4927,11 @@ rbreak_command (char *regexp, int from_tty)
> {
> struct symtab *symtab = symbol_symtab (p->symbol);
> const char *fullname = symtab_to_fullname (symtab);
> -
> - int newlen = (strlen (fullname)
> - + strlen (SYMBOL_LINKAGE_NAME (p->symbol))
> - + 4);
> -
> - if (newlen > len)
> - {
> - string = xrealloc (string, newlen);
> - len = newlen;
> - }
> - strcpy (string, fullname);
> - strcat (string, ":'");
> - strcat (string, SYMBOL_LINKAGE_NAME (p->symbol));
> - strcat (string, "'");
> + string = concat(fullname,
> + ":'",
> + SYMBOL_LINKAGE_NAME (p->symbol),
> + "'",
> + (char *)NULL);
> break_command (string, from_tty);
> print_symbol_info (FUNCTIONS_DOMAIN,
> p->symbol,
> @@ -4950,17 +4940,10 @@ rbreak_command (char *regexp, int from_tty)
> }
> else
> {
> - int newlen = (strlen (MSYMBOL_LINKAGE_NAME (p->msymbol.minsym)) + 3);
> -
> - if (newlen > len)
> - {
> - string = xrealloc (string, newlen);
> - len = newlen;
> - }
> - strcpy (string, "'");
> - strcat (string, MSYMBOL_LINKAGE_NAME (p->msymbol.minsym));
> - strcat (string, "'");
> -
> + string = concat("'",
> + MSYMBOL_LINKAGE_NAME (p->msymbol.minsym),
> + "'",
> + (char *)NULL);
> break_command (string, from_tty);
> printf_filtered ("<function, no debug info> %s;\n",
> MSYMBOL_PRINT_NAME (p->msymbol.minsym));
Hi.
Thanks for the patch.
Things are a bit odd, no disagreement there.
They are that way because note that all of this is
done in a loop, and we reuse the buffer for each
iteration.
The above patch will leak memory.
We *could* just concat the string for each iteration,
but we have to ensure the string is freed, even if, say,
break_command throws an error, so that means setting
up a cleanup inside the loop. At this point it's
probably a toss-up as to which is simpler, so I'd say
let's leave things as is.
Sound ok?
I feel silly for not seeing the for loop..... :-)
What you say makes sense.
I wish there was a concat in libiberty that would reuse the buffer for us.
Patch withdrawn.
On Sun, Jul 26, 2015 at 7:12 PM, Doug Evans <xdje42@gmail.com> wrote:
> Ciro Santilli <ciro.santilli@gmail.com> writes:
>> The function behaviour should be unchanged.
>>
>> len was always 0 , so I removed it.
>>
>> xrealloc was always called on NULL string, so equivalent to xmalloc,
>> which concat uses.
>>
>> ---
>> gdb/ChangeLog | 4 ++++
>> gdb/symtab.c | 35 +++++++++--------------------------
>> 2 files changed, 13 insertions(+), 26 deletions(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 407dfb4..3d8c416 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,7 @@
>> +4015-07-20 Ciro Santilli <ciro.santilli@gmail.com>
>> +
>> + * symtab.c (rbreak_command): Factor with concat.
>> +
>> 2015-07-15 Markus Metzger <markus.t.metzger@intel.com>
>> Pedro Alves <palves@redhat.com>
>>
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index decc5a9..015cb56 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -4889,7 +4889,6 @@ rbreak_command (char *regexp, int from_tty)
>> struct symbol_search *p;
>> struct cleanup *old_chain;
>> char *string = NULL;
>> - int len = 0;
>> const char **files = NULL;
>> const char *file_name;
>> int nfiles = 0;
>> @@ -4928,20 +4927,11 @@ rbreak_command (char *regexp, int from_tty)
>> {
>> struct symtab *symtab = symbol_symtab (p->symbol);
>> const char *fullname = symtab_to_fullname (symtab);
>> -
>> - int newlen = (strlen (fullname)
>> - + strlen (SYMBOL_LINKAGE_NAME (p->symbol))
>> - + 4);
>> -
>> - if (newlen > len)
>> - {
>> - string = xrealloc (string, newlen);
>> - len = newlen;
>> - }
>> - strcpy (string, fullname);
>> - strcat (string, ":'");
>> - strcat (string, SYMBOL_LINKAGE_NAME (p->symbol));
>> - strcat (string, "'");
>> + string = concat(fullname,
>> + ":'",
>> + SYMBOL_LINKAGE_NAME (p->symbol),
>> + "'",
>> + (char *)NULL);
>> break_command (string, from_tty);
>> print_symbol_info (FUNCTIONS_DOMAIN,
>> p->symbol,
>> @@ -4950,17 +4940,10 @@ rbreak_command (char *regexp, int from_tty)
>> }
>> else
>> {
>> - int newlen = (strlen (MSYMBOL_LINKAGE_NAME (p->msymbol.minsym)) + 3);
>> -
>> - if (newlen > len)
>> - {
>> - string = xrealloc (string, newlen);
>> - len = newlen;
>> - }
>> - strcpy (string, "'");
>> - strcat (string, MSYMBOL_LINKAGE_NAME (p->msymbol.minsym));
>> - strcat (string, "'");
>> -
>> + string = concat("'",
>> + MSYMBOL_LINKAGE_NAME (p->msymbol.minsym),
>> + "'",
>> + (char *)NULL);
>> break_command (string, from_tty);
>> printf_filtered ("<function, no debug info> %s;\n",
>> MSYMBOL_PRINT_NAME (p->msymbol.minsym));
>
> Hi.
>
> Thanks for the patch.
>
> Things are a bit odd, no disagreement there.
> They are that way because note that all of this is
> done in a loop, and we reuse the buffer for each
> iteration.
>
> The above patch will leak memory.
>
> We *could* just concat the string for each iteration,
> but we have to ensure the string is freed, even if, say,
> break_command throws an error, so that means setting
> up a cleanup inside the loop. At this point it's
> probably a toss-up as to which is simpler, so I'd say
> let's leave things as is.
>
> Sound ok?
@@ -1,3 +1,7 @@
+4015-07-20 Ciro Santilli <ciro.santilli@gmail.com>
+
+ * symtab.c (rbreak_command): Factor with concat.
+
2015-07-15 Markus Metzger <markus.t.metzger@intel.com>
Pedro Alves <palves@redhat.com>
@@ -4889,7 +4889,6 @@ rbreak_command (char *regexp, int from_tty)
struct symbol_search *p;
struct cleanup *old_chain;
char *string = NULL;
- int len = 0;
const char **files = NULL;
const char *file_name;
int nfiles = 0;
@@ -4928,20 +4927,11 @@ rbreak_command (char *regexp, int from_tty)
{
struct symtab *symtab = symbol_symtab (p->symbol);
const char *fullname = symtab_to_fullname (symtab);
-
- int newlen = (strlen (fullname)
- + strlen (SYMBOL_LINKAGE_NAME (p->symbol))
- + 4);
-
- if (newlen > len)
- {
- string = xrealloc (string, newlen);
- len = newlen;
- }
- strcpy (string, fullname);
- strcat (string, ":'");
- strcat (string, SYMBOL_LINKAGE_NAME (p->symbol));
- strcat (string, "'");
+ string = concat(fullname,
+ ":'",
+ SYMBOL_LINKAGE_NAME (p->symbol),
+ "'",
+ (char *)NULL);
break_command (string, from_tty);
print_symbol_info (FUNCTIONS_DOMAIN,
p->symbol,
@@ -4950,17 +4940,10 @@ rbreak_command (char *regexp, int from_tty)
}
else
{
- int newlen = (strlen (MSYMBOL_LINKAGE_NAME (p->msymbol.minsym)) + 3);
-
- if (newlen > len)
- {
- string = xrealloc (string, newlen);
- len = newlen;
- }
- strcpy (string, "'");
- strcat (string, MSYMBOL_LINKAGE_NAME (p->msymbol.minsym));
- strcat (string, "'");
-
+ string = concat("'",
+ MSYMBOL_LINKAGE_NAME (p->msymbol.minsym),
+ "'",
+ (char *)NULL);
break_command (string, from_tty);
printf_filtered ("<function, no debug info> %s;\n",
MSYMBOL_PRINT_NAME (p->msymbol.minsym));