On 03/15/2012 02:48 PM, Marc-André Lureau wrote:
> Peter,
>
> To resume the discussion and be a bit more specific:
>
>> +#if __GNUC__
>> +static inline std::string pretty_func_to_func_name(const std::string&
>> f_name)
>> +{
>> + std::string name(f_name);
>> + std::string::size_type end_pos = f_name.find('(');
>> + if (end_pos == std::string::npos)
>> + return f_name;
>> +
>> + std::string::size_type start = f_name.rfind(' ', end_pos);
>> + if (start == std::string::npos)
>> + return f_name;
>> +
>> + end_pos -= ++start;
>> + return name.substr(start, end_pos);
>> +}
>> +# define FUNC_NAME pretty_func_to_func_name(__PRETTY_FUNCTION__).c_str()
>> +#else
>> +# define FUNC_NAME __FUNCTION__
>> +#endif
>
> why not use only __FUNCTION__?
>
Because, it does not format the string to meet this pattern for classes:
"class::method". What I get is only the method name, which is not enough
in this case.
>>> - execl("/usr/libexec/spice-xpi-client",
>>> "/usr/libexec/spice-xpi-client", NULL);
>>> - g_message("failed to run spice-xpi-client, running spicec
>>> instead");
>>> + char *spice_xpi_client_args[] = {
>>> + const_cast<char*>("/usr/libexec/spice-xpi-client"),
>>> + NULL
>>> + };
>>> + std::stringstream ss;
>>> + int cnt = sizeof(spice_xpi_client_args) /
>>> sizeof(spice_xpi_client_args[0]);
>>> + for (int i = 0; i < cnt; ++i)
>>> + ss << spice_xpi_client_args[i] << " ";
>>> + LOG_MESSAGE("Launching " << ss.str());
>>> + execv(spice_xpi_client_args[0], spice_xpi_client_args);
>>> + LOG_MESSAGE("failed to run spice-xpi-client, running spicec
>>> instead");
>
> This change is quite intrusive and ugly imho, especially because you
> do it at least twice (here, +more for usb elsewhere).
>
It depends. I wanted to easily dump the parameters, which are passed to
the process - I wanted to avoid copying the same thing twice.
> Can't you define a exec helper instead, using a ostream operator<< for
> logging argv would be a plus, for instance.
>
>> /* attribute string Title; */
>> char *nsPluginInstance::GetTitle() const
>> {
>> + std::string title(m_title);
>> + size_t found = -2;
>> + while ((found = title.find("%", found + 2)) != std::string::npos)
>> + title.replace(found, 1, "%%");
>> + LOG_DEBUG(title);
>> return stringCopy(m_title);
>> }
>
> Hmm, I think I understand why you did this, because we use C
> printf-like g_log functions in the end, but we can't add use
> LOG_DEBUG("%s", title) sadly.
>
Yes, exactly!
> Maybe the g_log functions should be called like g_debug("%s", msg)
> instead, since they are not doing the formating anyway?
>
> thanks!
>
--
Peter Hatina
EMEA ENG-Desktop Development
Red Hat Czech, Brno
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
