On 2013.06.09 07:34, Hans de Goede wrote:> + if (i >= ARRAYSIZE(usbi_locale_supported)) {
> + usbi_warn(NULL, "Unrecognized locale format: %s", locale); > + return LIBUSB_ERROR_NOT_FOUND; > + } > > I wonder if we should warn here at all?
Considering that we are using NULL instead of ctx, which we probably want to avoid doing unless we really have too, I guess not. We want that call to work before libusb_init(), in case someone wants to print a localized strerror() on the value returned by init(), and the default context, which is what we'd use if NULL is passed, is created in init() => potentially bad results. Therefore, I removed the warning altogether. Calls that can be issued before init() should not use logging.
I also removed the comments about "en" being ASCII and anything else being UTF-8. US-ASCII is a subset of UTF-8, and people using localized calls should know that. Plus, on a sidenote: DIE, ASCII, DIE! If, in 2013, you're using ASCII rather than UTF-8, or still use software that can't handle UTF-8, you really oughta look into fixing that.
On 2013.06.09 10:03, Ludovic Rousseau wrote:
Remarks: - NULL is not a valid parameter for libusb_setlocale(). NULL is used if no -l parameter is used - You do not check the value returned by libusb_setlocale()
Yup. That was intentional, since I didn't consider localized strerror to be the prime focus of xusb.
Then again, since it's a sample, we probably want to give a example of good practices, so I modified the new commit to meet your proposal below:
Proposal: - call libusb_setlocale() _only_ when the 'l' argument is used - display the error message if libusb_setlocale() fails
However, I did not use your:
Code proposal:
case 'l':
r = libusb_setlocale(argv[++j]);
if (r != LIBUSB_SUCCESS) {
printf(" Option -l requires an ISO 639-1 language parameter");
return 1;
}
break;
1. I'd rather avoid having too many libusb calls issued before libusb_init(),. While some calls, such as libusb_get_version(), are fine (which we call first in xusb purely for cosmetic reasons), and the new version of libusb_setlocale() should also be good too, calls that can produce logging output should be avoided because of the default context => with libusb_setlocale did have an usbi_warn() then, I preferred to stay on the safe side.
Now, the new setlocale() should be safe to call before init(), but we don't use strerror() until after init, so I don't see much of a reason to do call setlocale() that early.
2. Your proposal dropped the part about checking whether the parameter following -l looked like an option flag ("-something" or "/something"). If someone tries something like "-l -i", I want to tell them that they probably missed the required parameter to -l, rather than try to pass "-i" as a locale.
See https://github.com/libusbx/libusbx/commit/511ed18228dd097dfe6d5c6fd926eaea24435f64#L0L1089 for more.
Regards, /PetePS: I'm also attaching the doxygen generated page that contains the content related to these calls, if you want to have a look. On a system with doxygen installed, you can also pick the latest and run 'make docs' in the doc directory.
group__misc.7z
Description: Binary data
------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. A cloud service to automate IT design, transition and operations 2. Dashboards that offer high-level views of enterprise services 3. A single system of record for all IT processes http://p.sf.net/sfu/servicenow-d2d-j
_______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
