Hi,
Thanks for the comments and the review.
On 05/28/2013 01:10 PM, Ludovic Rousseau wrote:
> Hello,
>
> 2013/5/28 Hans de Goede <hdego...@redhat.com>:
>> This patch adds the much requested libusb_strerror() function, taking into
>> account all issues people raised wrt previous attempts.
>>
>> Criteria / Decisions underlying this implementation:
>> -Must support translated messages
>> -Must not use gettext as that does not work well in combination with Windows
>> (when building with Visual C, or for Windows CE)
>> -API compatible with FreeBSD and various patched libusb-s floating around
>> -KISS:
>> -Do not add any (other) library dependencies
>> -Do not try to deal with message encodings (iconv), simply always return
>> UTF-8
>> making encoding the problem of the application using libusb_strerror.
>> Apps which don't want to deal with encoding can opt-out with a simple:
>> libusb_set_strerror_locale("en") call, after which the messages returned
>> by
>> libusb_strerror are guaranteed to be pure ASCII.
>
> In my case I will use libusb_strerror() to get a human readable
> message in the log trace.
> Since the log is for me (and not for the end user) I do not want to
> get a message in Dutch so I will call libusb_set_strerror_locale("en")
>
> libusb_set_strerror_locale("en") is not only used to avoid UTF-8 problems.
Right, it can of course also be used to get English messages if you want
English messages. But that is its "regular" use. I specifically mention the
UTF-8 case because it is important for people to be aware that they either
need to deal with UTF-8 text, or use English messages.
>
> I have 2 comments bellow:
>
<snip>
>> +/** \ingroup misc
>> + * Set the language, and only the language, not the encoding! used for
>> + * libusb_strerror messages. This takes a locale string in the default
>> + * setlocale format. Both Windows and Posix locale strings are excepted, ie
>
> excepted -> accepted.
I had already noticed this myself, and it is already fixed in my local copy.
<snip>
>> + case LIBUSB_ERROR_INTERRUPTED:
>> + return "Onderbroken systeemaanroep";
>> + case LIBUSB_ERROR_NO_MEM:
>> + return "Onvoldoende geheugen beschikbaar";
>> + case LIBUSB_ERROR_NOT_SUPPORTED:
>> + return "Bewerking wordt niet ondersteund";
>> + case LIBUSB_ERROR_OTHER:
>> + return "Andere fout";
>> + default:
>> + return NULL;
>
> The English version returns "Unknown error" instead of NULL. I would
> not like to get a NULL pointer instead of a valid C-string.
>
> I now understand why after reading the code of libusb_strerror().
> Maybe "Unknown error" should also be translated. No?
> Or maybe add a comment like /* fall back to English translation */
> before the return NULL;
Right, the idea is that as new error-codes get added, and only an
English message gets added for them initially, that it is then better
to show the English msg for the new error-code, rather then a
translated "Unknown error"
Granted this means that in case of a truely unknown error, the user
will see "Unknown Error" instead of for example "Onbekende Fout", but
the Unknown Error case should not happen, so I believe returning NULL
in the default case for any language but English is best.
I'll add the comment you suggested to clarify things.
Regards,
Hans
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
