[Spice-devel] spice-xpi

[Marc-André Lureau]

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__?

>> -        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).

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.

Maybe the g_log functions should be called like g_debug("%s", msg)
instead, since they are not doing the formating anyway?

thanks!

-- 
Marc-André Lureau