Ticket #27 (closed defect: fixed)
Opened 3 years ago
Last modified 3 years ago
Libusb 1.0.6 Big Endian issue
| Reported by: | mig_143 | Owned by: | |
|---|---|---|---|
| Milestone: | Component: | libusb-1.0 | |
| Keywords: | Big Endian | Cc: | |
| Blocked By: | Blocks: |
Description
Hi,
I am running Libusb 1.0.6 on x86 (little endian) board. It works fine.
But when I run the same on Mips (Big Endian) it doesn't works.
I investigated and found in Descriptor.c file functions like usbi_parse_descriptor, parse_endpoint, parse_interface ... etc. seems to have endian flag (int host_endian) in function arguments, but calling functions are not taking care about the endian issue.
I searched the internet and got one suggestion
I added the following snippet, in above said functions
#ifdef BIG_ENDIAN
host_endian = 0;
#else
host_endian = 1;
#endif
and after that libusb is working fine.
Libusb should handle this also.
Thanks
Mohit
code snippet ---->
int usbi_parse_descriptor(unsigned char *source, char *descriptor, void *dest,
int host_endian)
{
unsigned char *sp = source, *dp = dest;
uint16_t w;
char *cp;
#ifdef BIG_ENDIAN
host_endian = 0;
#else
host_endian = 1;
#endif
for (cp = descriptor; *cp; cp++) {
switch (*cp) {
case 'b': /* 8-bit byte */
*dp++ = *sp++;
break;
case 'w': /* 16-bit word, convert from little endian to CPU */
dp += ((unsigned long)dp & 1); /* Align to word boundary */
if (host_endian) {
memcpy(dp, sp, 2);
} else {
}
sp += 2;
dp += 2;
break;
}
}
return sp - source;
}
Change History
comment:1 Changed 3 years ago by mig_143
- Summary changed from Libusb 1.0.6 don't caring about big endian issue to Libusb 1.0.6 Big Endian issue
comment:2 follow-up: ↓ 3 Changed 3 years ago by dsd
comment:3 in reply to: ↑ 2 Changed 3 years ago by mig_143
Replying to dsd:
What do you mean by "it doesn't work" ?
It does not enumerate the device and returns error.
That happens because it is not handling big-endian issue properly.
Like in our case device descriptor lenth is 0x19 (25) bytes, but because while parsing the device descriptor it issues a request for 0x1900 (6400) bytes.
Device returns 25 bytes but it returns error saying --->
libusb:error [get_config_descriptor] short output read 25/6400.
You can also verify the same, running Libusb on some big endian machine.
Also access this link-->
http://old.nabble.com/build-libusb-for-big-endian-target---how-to-and-a-recommended-patch-td22956999.html
Further see my previous comments for solution I have for this.
Thanks
Mohit
comment:4 Changed 3 years ago by mig_143
Today I find a better way to put this patch... At run time it will check the plaform for big-endian and little endian.
If I am adding only this patch, in usbi_parse_descriptor function in descriptor.c file, all the problems are solved.
I found in the code, many functions are calling usbi_parse_descriptor with a hardcoded valude for host_endian (0 or 1).
Please check the following code snippet-->
int usbi_parse_descriptor(unsigned char *source, char *descriptor, void *dest,
int host_endian)
{
unsigned char *sp = source, *dp = dest;
uint16_t w;
char *cp;
#ifdef GET_ENDIAN
w = 1;
host_endian = *(unsigned char *)(& w);
#endif
for (cp = descriptor; *cp; cp++) {
switch (*cp) {
case 'b': /* 8-bit byte */
*dp++ = *sp++;
break;
Thanks
Mohit
comment:5 Changed 3 years ago by dsd
The host_endian flag means that we expect the data to be in the endianness of the host, so conversion is never necessary. If this is not correct for a particular case then the caller should be fixed, we should not break the concept of host endianness for every single caller.
If you look at the comments in the file you can see it is a bit of a delicate area and there are multiple codepaths for reading the descriptor info, so this needs looking at in more detail. It's unlikely that the correct fix would touch the usbi_parse_descriptor() function.
comment:6 Changed 3 years ago by mig_143
ya you are right. callers should be fixed for that. As you told multiple codepaths are there for reading the descriptor info, they should pass the correct value for host_endian parameter while calling the "usbi_parse_descriptor" function.
But they are not doing that. And all those callers are part of libusb code. In most of the places they are passing hard coded value (0 or 1). So lot of changes will have to made. So for fixing it with the smallest code possible I tried putting in the usbi_parse_descriptor function.
I hope in next release of libusb this issue will be taken care
comment:7 Changed 3 years ago by dsd
It seems like you don't understand what is meant by host endian - it means that the data is expected to be already in the endianness of the host, therefore no conversion is necessary *on any system* regardless of its endianness. Because host endianness is not something that varies by platform, it is intentional that those 0s and 1s are hardcoded. What you are probably seeing is a small detail where we have overlooked a small detail and there is a 1 instead of a 0 or something like that.
comment:8 Changed 3 years ago by mig_143
So according to you no conversion is required.
Can you tell me, if I have to run libusb 1.0.6 on a Big-Endian machine do I need to configure any flag or option before compiling the code ?
comment:9 Changed 3 years ago by dsd
I'm saying that there is somewhere in the code that is doing a conversion where none is required, or not doing a conversion where one is required. This will likely just be a 0 or 1 flag set incorrectly. Figuring out which callsite is the source of this is the first step towards the solutoin.
comment:10 Changed 3 years ago by mig_143
I have removed the patch from usbi_parse_descriptor function (descriptor.c file), as I suggested earlier.
I have modified in Linux_usbfs.c file. In function -->
static int cache_active_config(struct libusb_device *dev, int fd, int active_config)
usbi_parse_descriptor is called with host_endian value as 1 -->
usbi_parse_descriptor(tmp, "bbw", &config, 1);
I changed it to 0--> usbi_parse_descriptor(tmp, "bbw", &config, 0);
and it works on both the Hardware platforms i.e. Little Endian as well Big-Endian.
comment:11 Changed 3 years ago by dsd
yep, thats half of it. Should be fully fixed in commit 02df59a309e813c50b8230de99e69fb4e1814279 for the next release
comment:12 Changed 3 years ago by dsd
- Resolution set to fixed
- Status changed from new to closed
fixed in v1.0.7
What do you mean by "it doesn't work" ?