Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Trevor Saunders <tbsaunde@tbsaunde.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2 01/31] Introduce string_printf
Date: Wed, 19 Oct 2016 14:41:00 -0000	[thread overview]
Message-ID: <37034a83-e419-3fba-19f7-9974b62cb693@redhat.com> (raw)
In-Reply-To: <20161019135300.37ghx3zsjincxwh7@ball>

On 10/19/2016 02:53 PM, Trevor Saunders wrote:
>> +std::string
>> +string_printf (const char* fmt, ...)
> 
> std::string is unfortunately kind of large for a return value
> its 4 * sizeof (void *), but maybe the simplicity matters more here.

That shouldn't matter I think?  Beyond a size, ABIs will return via
the stack instead, and, also, no actual copying (in C++ sense) will
take place, because NRVO should apply here.

> 
>> +{
>> +  std::string str;
>> +  va_list vp;
>> +
>> +  /* Start by assuming some reasonable size will be sufficient.  */
>> +  str.resize (1024);
>> +
>> +  while (1)
>> +    {
>> +      size_t size;
>> +      int result;
> 
> you could declare these at first use right?

Yeah, I considered it, and went back and forth, actually!

It'd look like this instead:

  while (1)
    {
      va_start (vp, fmt);
      size_t size = str.size ();
      int result = vsnprintf (&str[0], size, fmt, vp);
      va_end (vp);

      str.resize (result);

      if (result < size)
	break;
    }

Felt odd to me to see the variables declared in the middle of
va_start/va_end, since those kind of form a scope.  I guess I'm
still not used to declaring PODs i the middle of blocks.  If we
don't that, then when a variable is declared in the middle of a
scope it really stands out that something important might be going
on with the ctor.  But maybe that's just me?  I can certainly
change it. 

Also, while writing this, I was also going back and forth between
doing the initial reserve:

 +  str.resize (1024);

and not doing it, and instead call vsnprintf with a NULL buffer
to pre-compute the necessary size.  The downsize is that
you always have to call vsnprintf twice that way.  The upside
is less wasted memory, considering the case of these strings
ending up stored in some structures.  I saw today that
xstrprintf (via vasprintf) uses the 'pre-compute necessary size'
instead of the 'pre-reserve' approach, so I'm leaning toward
doing that too now.

Thanks,
Pedro Alves


  reply	other threads:[~2016-10-19 14:41 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-19  1:13 [PATCH v2 00/31] More cleanup elimination & unlimited args to user-defined funcs Pedro Alves
2016-10-19  1:12 ` [PATCH v2 19/31] Use ui_file_as_string in gdb/remote.c Pedro Alves
2016-10-19  1:12 ` [PATCH v2 01/31] Introduce string_printf Pedro Alves
2016-10-19 13:43   ` Trevor Saunders
2016-10-19 14:41     ` Pedro Alves [this message]
2016-10-19 17:18   ` Simon Marchi
2016-10-19 21:02     ` Pedro Alves
2016-11-08 15:35       ` Pedro Alves
2016-10-19  1:12 ` [PATCH v2 21/31] Use ui_file_as_string in gdb/compile/ Pedro Alves
2016-10-19 23:08   ` Simon Marchi
2016-10-19 23:48     ` Pedro Alves
2016-10-20  3:17       ` Simon Marchi
2016-10-19  1:12 ` [PATCH v2 22/31] Use ui_file_as_string in gdb/c-exp.y Pedro Alves
2016-10-19  1:12 ` [PATCH v2 07/31] Clean up tracepoint.h/c:collection_list Pedro Alves
2016-10-19  1:12 ` [PATCH v2 24/31] Use ui_file_as_string in gdb/ada-lang.c Pedro Alves
2016-10-19  1:12 ` [PATCH v2 06/31] Introduce ui_file_as_string Pedro Alves
2016-10-19  1:12 ` [PATCH v2 20/31] Use ui_file_as_string in gdb/cli/cli-setshow.c Pedro Alves
2016-10-19  1:12 ` [PATCH v2 12/31] Use ui_file_as_string in gdb/utils.c Pedro Alves
2016-10-19  1:13 ` [PATCH v2 27/31] Use ui_file_as_string in gdb/language.c Pedro Alves
2016-10-19  1:13 ` [PATCH v2 03/31] breakpoint.c:commands_command_1 constification and cleanup Pedro Alves
2016-10-19  1:13 ` [PATCH v2 02/31] cli/cli-script.c: Remove some dead NULL checks Pedro Alves
2016-10-19 17:24   ` Simon Marchi
2016-10-19 21:18     ` Pedro Alves
2016-10-19  1:13 ` [PATCH v2 26/31] Use ui_file_as_string in gdb/rust-lang.c Pedro Alves
2016-10-19  1:13 ` [PATCH v2 08/31] Use ui_file_as_string in dwarf2_compute_name Pedro Alves
2016-10-19  1:13 ` [PATCH v2 29/31] 'struct agent_expr *' -> unique_ptr<agent_expr> Pedro Alves
2016-10-19 23:19   ` Simon Marchi
2016-10-19 23:58     ` Pedro Alves
2016-10-19  1:13 ` [PATCH v2 14/31] Use ui_file_as_string in gdb/guile/ Pedro Alves
2016-10-19  1:13 ` [PATCH v2 15/31] Use ui_file_as_string in execute_command_to_string Pedro Alves
2016-10-19  1:13 ` [PATCH v2 31/31] Support an "unlimited" number of user-defined arguments Pedro Alves
2016-10-19  6:29   ` Eli Zaretskii
2016-10-19 11:33   ` Philipp Rudo
2016-10-19 12:47     ` Pedro Alves
2016-10-19 17:40       ` Philipp Rudo
2016-10-19 17:45         ` Pedro Alves
2016-11-08 15:41   ` Pedro Alves
2016-10-19  1:13 ` [PATCH v2 23/31] Use ui_file_as_string in gdbarch.sh/gdbarch.c Pedro Alves
2016-10-19  1:17 ` [PATCH v2 25/31] Use ui_file_as_string in gdb/infrun.c Pedro Alves
2016-10-19  1:17 ` [PATCH v2 09/31] Use ui_file_as_string in gdb/xtensa-tdep.c Pedro Alves
2016-10-19  1:17 ` [PATCH v2 10/31] Use ui_file_as_string in gdb/ada-valprint.c Pedro Alves
2016-10-19  1:17 ` [PATCH v2 11/31] Use ui_file_as_string in gdb/ui-out.c Pedro Alves
2016-10-19  1:18 ` [PATCH v2 30/31] Eliminate agent_expr_p; VEC -> std::vector in struct bp_target_info Pedro Alves
2016-10-19  1:18 ` [PATCH v2 05/31] 'struct expression *' -> gdb::unique_xmalloc_ptr<expression> Pedro Alves
2016-10-19 18:45   ` Simon Marchi
2016-10-19 21:50     ` Pedro Alves
2016-10-19 22:25       ` Simon Marchi
2016-10-19 22:36         ` Pedro Alves
2016-10-19  1:19 ` [PATCH v2 04/31] cli-script.c: Simplify using std::string, eliminate cleanups Pedro Alves
2016-10-19 18:25   ` Simon Marchi
2016-10-19 21:45     ` Pedro Alves
2016-10-19  1:21 ` [PATCH v2 17/31] Use ui_file_as_string in gdb/printcmd.c Pedro Alves
2016-10-19  1:21 ` [PATCH v2 16/31] Use ui_file_as_string in gdb/top.c Pedro Alves
2016-10-19  1:21 ` [PATCH v2 18/31] Use ui_file_as_string in gdb/python/ Pedro Alves
2016-10-19  1:21 ` [PATCH v2 13/31] Use ui_file_as_string in gdb/arm-tdep.c Pedro Alves
2016-10-19 22:54   ` Simon Marchi
2016-10-20 13:08 ` [PATCH v2 28/31] Use ui_file_as_string throughout more Pedro Alves
2017-02-23 10:23   ` Yao Qi
2017-02-23 10:53     ` Pedro Alves
2017-02-23 10:35   ` Yao Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37034a83-e419-3fba-19f7-9974b62cb693@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tbsaunde@tbsaunde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox