[PATCH v2] Remove warning 'format not a string literal' in libsvn_client

[Daniel Näslund <da...@longitudo.com>]

On Thu, Nov 05, 2009 at 11:19:37AM +0000, Julian Foad wrote:
[[[
* subversion/libsvn_client/diff.c
  (display_prop_diffs): Remove warnings about 'format not a string
    literal'.

* subversion/libsvn_client/export.c
  (copy_one_versioned_file): Remove warnings about 'format not a string
    literal'.

Patch by: Daniel Näslund <da...@longitudo.com>
          Julian Foad
]]]

> For this bit:
> 
> >        if (! original_value)
> > -        header_fmt = _("Added: %s%s");
> > +        SVN_ERR(file_printf_from_utf8(file, encoding, "Added: %s%s",
> > +                                    propchange->name, APR_EOL_STR));
> >        else if (! propchange->value)
> > -        header_fmt = _("Deleted: %s%s");
> > +        SVN_ERR(file_printf_from_utf8(file, encoding, "Deleted: %s%s",
> > +                                    propchange->name, APR_EOL_STR));
> >        else
> > -        header_fmt = _("Modified: %s%s");
> > -      SVN_ERR(file_printf_from_utf8(file, encoding, header_fmt,
> > +        SVN_ERR(file_printf_from_utf8(file, encoding, "Modified: %s%s",
> >                                      propchange->name, APR_EOL_STR));
> 
> consider writing:
> 
>        if (! original_value)
>          action = _("Added");
>        else if (! propchange->value)
>          action = _("Deleted");
>        else
>          action = _("Modified");
>        SVN_ERR(file_printf_from_utf8(file, encoding,
>                                      "%s: %s" APR_EOL_STR,
>                                      action, propchange->name));
> 
> and similarly, in the other bit, you could do:
> 
> [... suffix = "M" or "" ...]
> 
>        SVN_ERR(svn_subst_build_keywords2
>                (&kw, keywords->data,
> -               apr_psprintf(scratch_pool, fmt, changed_rev),
> +               apr_psprintf(scratch_pool, "%ld%s",
> +                            changed_rev, suffix),
>                 entry->url, tm, author, scratch_pool));

That was so much better. Very few lines in the previous patch but I
managed to screw up a couple of localisation strings and do some heavy
code duplication. Not bad.

I've run the export_tests and diff_tests. They pass.

/Daniel

Re: [PATCH v2] Remove warning 'format not a string literal' in libsvn_client

[Julian Foad <ju...@btopenworld.com>]

Daniel Näslund wrote:
> On Thu, Nov 05, 2009 at 11:19:37AM +0000, Julian Foad wrote:
> [[[
> * subversion/libsvn_client/diff.c
>   (display_prop_diffs): Remove warnings about 'format not a string
>     literal'.
> 
> * subversion/libsvn_client/export.c
>   (copy_one_versioned_file): Remove warnings about 'format not a string
>     literal'.
> 
> Patch by: Daniel Näslund <da...@longitudo.com>
>           Julian Foad
> ]]]
[...]
> That was so much better. Very few lines in the previous patch but I
> managed to screw up a couple of localisation strings and do some heavy
> code duplication. Not bad.
> 
> I've run the export_tests and diff_tests. They pass.

Thanks! Committed revision 40389.

I tried to write the log message in a way that doesn't imply that
removing warnings is the "be-all and end-all" of why we want this
change. In other words, the warnings gave us a hint that we might like
to make an improvement to our code, but we made the change because it is
an improvement, not just because it stops the warnings from appearing.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414883