Improve relocation debug messages.

Improve relocation debug messages.

This patch contains the following commits.

* relocate.cc: Don't use double forward slashes in file names.

* main.cc, relocate.cc: Improve relocation debug messages.

  This commit does the following.

  * Add `_f ()' where necessary.
  * Improve readability of code by omitting some `.c_str ()' conversions.
  * Synchronize formatting of messages.
  * Improve messages to make the more readable.  Also add some new ones.
  * Indent all messages related to relocation.

* relocate.cc: Add `indent' parameter to `sane_putenv'.

  We are going to use that to provide better formatting of debug messages.

[dak, 2019-02-08 09:42:27]

https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc
File lily/relocate.cc (right):

https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128
lily/relocate.cc:128: TOPLEVEL_VERSION, package_datadir));
Why no .c_str () here (and lots of other places)?

[lemzwerg, 2019-02-08 09:49:58]

On 2019/02/08 09:42:27, dak wrote:
> https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc
> File lily/relocate.cc (right):
> 
> https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128
> lily/relocate.cc:128: TOPLEVEL_VERSION, package_datadir));
> Why no .c_str () here (and lots of other places)?

Why should it?  It works without `c_str'.  Am I missing something?

[dak, 2019-02-08 10:36:44]

On 2019/02/08 09:49:58, lemzwerg wrote:
> On 2019/02/08 09:42:27, dak wrote:
> > https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc
> > File lily/relocate.cc (right):
> > 
> > https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128
> > lily/relocate.cc:128: TOPLEVEL_VERSION, package_datadir));
> > Why no .c_str () here (and lots of other places)?
> 
> Why should it?  It works without `c_str'.  Am I missing something?

Well, we do prefer code that works by design rather than code that works by mere
chance and due to idiosyncrasies of the GNU C++ library.  _f needs
zero-terminated strings for %s format fields but being a C-style format string,
cannot enforce the respective conversions.

Can you give any reason why it _should_ work rather than "I tried"?

[lemzwerg, 2019-02-08 11:41:45]

> Can you give any reason why it _should_ work rather than "I tried"?

flower/include/international.hh:46

[dak, 2019-02-08 12:05:10]

On 2019/02/08 11:41:45, lemzwerg wrote:
> > Can you give any reason why it _should_ work rather than "I tried"?
> 
> flower/include/international.hh:46

Oh good grief.

string _f (char const *format, ...)
__attribute__ ((format (printf, 1, 2)));
string _f (char const *format, const string &s, const string &s2 = "", const
string &s3 = "");

So this is a format-like string unless there follow 1 to 3 C++ strings (or types
convertible to strings) in which case GCC does not get to check argument
consistency.  And the code then does

string
_f (char const *format, const string &s, const string &s2, const string &s3)
{
  return String_convert::form_string (gettext (format), s.c_str (), s2.c_str (),
                                      s3.c_str ());
}

anyway.  Frankly, my approach would be to throw that function out.  It just
obfuscates what will and what will not work and keeps GCC from issuing warnings
when number and uses of %s don't match.

[lemzwerg, 2019-02-08 12:14:58]

> Frankly, my approach would be to throw that function out.  It just
> obfuscates what will and what will not work and keeps GCC from issuing
> warnings when number and uses of %s don't match.

Hmm.  Having a C++ version of `_f' is actually quite nice, since appending
`c_str' again and again is a nuisance IMHO.  Could you come up with a better
replacement?  If this is not possible please go ahead and clean up the code.

[dan_faithful.be, 2019-02-08 22:46:18]

On Feb 8, 2019, at 07:05, dak@gnu.org wrote:
> 
> string
> _f (char const *format, const string &s, const string &s2, const string
> &s3)
> {
>  return String_convert::form_string (gettext (format), s.c_str (),
> s2.c_str (),
>                                      s3.c_str ());
> }
> 
> anyway.  Frankly, my approach would be to throw that function out.  It
> just obfuscates what will and what will not work and keeps GCC from
> issuing warnings when number and uses of %s don't match.

+1 (but I’m not volunteering)
— 
Dan