[UrJTAG-dev] libftdi VID=/PID= runtime arguments are ignored?

[Elden Tyrell]

This has been bugging me for some time...

Has anybody noticed that you can't actually use the VID/PID arguments to override the built-in USB ids for libftdi-based cables? This is really unfortunate, because a lot of cables out there are just knock-offs of some existing urjtag-supported cable with the VID/PID changed.

Here's an example: I have an Olimex ARM-USB-OCD-H (FT2232H).

$ lsusb | grep -i olimex
Bus 002 Device 006: ID 15ba:002a Olimex Ltd.

It isn't recognized by urjtag svn-2026 as either an ARM-USB-OCD-H or a gnICE+:

UrJTAG 0.10 #2026
Copyright (C) 2002, 2003 ETC s.r.o.
...
jtag> cable ARM-USB-OCD-H
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found
...
jtag> cable gnICE+
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found

jtag> cable gnICE+ VID=15ba PID=002a
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found

On the other hand if I change the VID and PID in the source code like this:

svn diff src/tap
Index: src/tap/cable/ft2232.c
===================================================================
--- src/tap/cable/ft2232.c (revision 2026)
+++ src/tap/cable/ft2232.c (working copy)
@@ -2353,7 +2353,7 @@
ft2232_flush,
ftdx_usbcable_help
};
-URJ_DECLARE_FTDX_CABLE(0x0456, 0xF001, "-mpsse", "gnICE+", gniceplus)
+URJ_DECLARE_FTDX_CABLE(0x15ba, 0x002a, "-mpsse", "gnICE+", gniceplus)

const urj_cable_driver_t urj_tap_cable_ft2232_jtagkey_driver = {
"JTAGkey",

... it works perfectly!

jtag> cable gnICE+
Connected to libftdi driver.
jtag> idcode
Reading 0 bytes of idcode
Read 10010011(0x93) 11010000(0xd0) 00000001(0x01) 00110100(0x34) 00000000(0x00) 00000000(0x00) 00000000(0x00) 00000000(0x00)

So, my question: why are the runtime VID/PID parameters ignored?

Thanks,

- e

[Kees Jongenburger]

HI,

I don't know if this will help but the help page on your calbe states
you need to use lowercase characters

jtag> cable gnICE+ help
Usage: cable gnICE+ [vid=VID] [pid=PID] [desc=DESC]
[interface=INTERFACE] [driver=DRIVER]

VID USB Device Vendor ID (hex, e.g. 0abc)
PID USB Device Product ID (hex, e.g. 0abc)
DESC Some string to match in description or serial no.
INTERFACE Interface to use (0=first, 1=second, etc).
DRIVER usbconn driver, either ftdi-mpsse or ftd2xx-mpsse

Default: vid=456 pid=f001 driver=ftdi-mpsse

try:
cable gnICE+ vid=15ba pid=002a

Greetings

Post by Elden Tyrell
This has been bugging me for some time...
Has anybody noticed that you can't actually use the VID/PID arguments to override the built-in USB ids for libftdi-based cables? This is really unfortunate, because a lot of cables out there are just knock-offs of some existing urjtag-supported cable with the VID/PID changed.
Here's an example: I have an Olimex ARM-USB-OCD-H (FT2232H).
$ lsusb | grep -i olimex
Bus 002 Device 006: ID 15ba:002a Olimex Ltd.
UrJTAG 0.10 #2026
Copyright (C) 2002, 2003 ETC s.r.o.
...
jtag> cable ARM-USB-OCD-H
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found
...
jtag> cable gnICE+
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found
jtag> cable gnICE+ VID=15ba PID=002a
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found
svn diff src/tap
Index: src/tap/cable/ft2232.c
===================================================================
--- src/tap/cable/ft2232.c (revision 2026)
+++ src/tap/cable/ft2232.c (working copy)
@@ -2353,7 +2353,7 @@
ft2232_flush,
ftdx_usbcable_help
};
-URJ_DECLARE_FTDX_CABLE(0x0456, 0xF001, "-mpsse", "gnICE+", gniceplus)
+URJ_DECLARE_FTDX_CABLE(0x15ba, 0x002a, "-mpsse", "gnICE+", gniceplus)
const urj_cable_driver_t urj_tap_cable_ft2232_jtagkey_driver = {
"JTAGkey",
... it works perfectly!
jtag> cable gnICE+
Connected to libftdi driver.
jtag> idcode
Reading 0 bytes of idcode
Read 10010011(0x93) 11010000(0xd0) 00000001(0x01) 00110100(0x34) 00000000(0x00) 00000000(0x00) 00000000(0x00) 00000000(0x00)
So, my question: why are the runtime VID/PID parameters ignored?
Thanks,
- e
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
UrJTAG-development mailing list
https://lists.sourceforge.net/lists/listinfo/urjtag-development

[Elden Tyrell]

Post by Kees Jongenburger
I don't know if this will help but the help page on your calbe states
you need to use lowercase characters
cable gnICE+ vid=15ba pid=002a

Nope, same result:

jtag> cable gnICE+ vid=15ba pid=002a
error: Couldn't connect to suitable USB device.
error: ftdi/ftd2xx error: ftdi_usb_open_desc() failed: device not found

[Mike Frysinger]

Post by Elden Tyrell
jtag> cable gnICE+

i doubt gnICE is what you want

Post by Elden Tyrell
jtag> cable gnICE+ VID=15ba PID=002a

if you use 0x15ba, does it work better ?
-mike

[Elden Tyrell]

Post by Mike Frysinger

Post by Elden Tyrell
jtag> cable gnICE+ VID=15ba PID=002a

if you use 0x15ba, does it work better ?

Yep, that works:

jtag> cable gnICE+ vid=0x15ba pid=0x002a
Connected to libftdi driver.

There is a bug in the error checking code in params.c that causes it to silently ignore sscanf() errors so long as at least a single character is consumed; in my case it stopped after "15" and "0002" rather than complain about the fact that "15ba" and "002a" are not decimal numbers.

The attached patch causes it to parse the entire input rather than an arbitrary nonempty prefix of it.

- e

[Elden Tyrell]

Post by Elden Tyrell

Post by Mike Frysinger

Post by Elden Tyrell
jtag> cable gnICE+ VID=15ba PID=002a

if you use 0x15ba, does it work better ?

jtag> cable gnICE+ vid=0x15ba pid=0x002a
Connected to libftdi driver.
There is a bug in the error checking code in params.c that causes it to silently ignore sscanf() errors so long as at least a single character is consumed; in my case it stopped after "15" and "0002" rather than complain about the fact that "15ba" and "002a" are not decimal numbers.
The attached patch causes it to parse the entire input rather than an arbitrary nonempty prefix of it.

Sorry, please use this one instead.

- e

[Mike Frysinger]

Post by Elden Tyrell

Post by Mike Frysinger

Post by Elden Tyrell
jtag> cable gnICE+ VID=15ba PID=002a

if you use 0x15ba, does it work better ?

jtag> cable gnICE+ vid=0x15ba pid=0x002a
Connected to libftdi driver.
There is a bug in the error checking code in params.c that causes it to
silently ignore sscanf() errors so long as at least a single character is
consumed; in my case it stopped after "15" and "0002" rather than complain
about the fact that "15ba" and "002a" are not decimal numbers.
The attached patch causes it to parse the entire input rather than an
arbitrary nonempty prefix of it.

i'm not sure how this can work. POSIX states for %n:

No input is consumed. The application shall ensure that the corresponding
argument is a pointer to the integer into which shall be written the number of
bytes read from the input so far by this call to the fscanf() functions.
Execution of a %n conversion specification shall not increment the assignment
count returned at the completion of execution of the function. No argument
shall be converted, but one shall be consumed. If the conversion specification
includes an assignment-suppressing character or a field width, the behavior is
undefined.

so %n is telling us how many *bytes* were read, not the number of elements
consumed. further, it's not telling us how things are being interpreted --
dec, hex, oct, whatever. we made a conscious decision to only support hex
numbers that start with 0x to avoid ambiguity. if there are places where the
documentation can be made more clear, then we should update that.

back to the patch at hand, if we look at two cases here, we see it doesn't
actually work:
sscanf("0a", "%lu%n", lu, &dummy);
- lu = 0 (wrong)
- "a" left unconsumed in the input string (wrong)
- dummy = 1
- return value = 1

sscanf("0111", "%lu%n", lu, &dummy);
- lu = 111 (wrong)
- input string completely consumed
- dummy = 4
- return value = 1

i'm guessing what you actually want is:
int dummy;
if (sscanf(eq, "%lu%n", lu, &dummy) == 1 && eq[dummy] == '\0')
return URJ_STATUS_OK;

however, i'd point out that this only catches errors where the user supplied a
hex value that contains non-numbers. it can't tell that when the user entered
"0111", they actually meant "0x111". or maybe they did actually mean "0111",
so this core function shouldn't be rejecting it in the first place.

maybe as part of the cable output, we can echo back "forcing VID=0x1234" so
that they can immediately tell "oh, my number is being interpreted as a
decimal when i really want hex, so i need to add the 0x prefix".
-mike

[Elden Tyrell]

Post by Mike Frysinger
so %n is telling us how many *bytes* were read, not the number of elements
consumed.
if (sscanf(eq, "%lu%n", lu, &dummy) == 2)
sscanf("0a", "%lu%n", lu, &dummy);
- return value = 1

and 1!=2

Perhaps a clearer alternative would be:

if (sscanf(eq, "%lu%c", lu, &dummy) == 1)

If there is junk left over the %c will match and it will return 2, and 2!=1.

For the record I am not a huge fan of the libc parsing/printing functions. It's been at least five years since I last used them in code I wrote myself. If you have a better way to check that the input was fully consumed, that's fine with me.

Post by Mike Frysinger
however, i'd point out that this only catches errors where the user supplied a
hex value that contains non-numbers.

Yes, and that's the only situation where the user -- such as me -- is thinking to themselves "well, I gave it something with letters in it, and it didn't complain, so it can't possibly be parsing it as decimal or octal!"

Seriously, this is the reason why I never bothered trying a "0x" or "16'h" prefix.

Post by Mike Frysinger
maybe as part of the cable output, we can echo back "forcing VID=0x1234" so
that they can immediately tell "oh, my number is being interpreted as a
decimal when i really want hex, so i need to add the 0x prefix".

That would be fine too.

- e

[Mike Frysinger]

Post by Mike Frysinger
so %n is telling us how many *bytes* were read, not the number of
elements consumed.

Yes, but the *return value* is the "the number of input *items* assigned"

(man sscanf); that's why I check that it's equal to 2 indicating that parsing
didn't fail before reaching %n:

please read the documentation i quoted, as well as run the examples i posted.
it does *not* return two.

if (sscanf(eq, "%lu%c", lu, &dummy) == 1)
If there is junk left over the %c will match and it will return 2, and 2!=1.

that would also work

Post by Mike Frysinger
however, i'd point out that this only catches errors where the user
supplied a hex value that contains non-numbers.

Yes, and that's the only situation where the user -- such as me -- is
thinking to themselves "well, I gave it something with letters in it, and
it didn't complain, so it can't possibly be parsing it as decimal or
octal!"
Seriously, this is the reason why I never bothered trying a "0x" or "16'h" prefix.

i think you missed the point -- yes, we should catch underparsed hex values,
but it wouldn't completely solve the problem. if your vid/pid happened to be
fully numeric, then you still wouldn't have figured out that 0x was needed.
-mike

[Mike Frysinger]

i've committed this:
--- src/global/params.c (revision 2026)
+++ src/global/params.c (working copy)
@@ -189,7 +189,9 @@ parse_param_lu(const char *eq, long unsi
*/
if (strncmp(eq, "0x", 2))
{
- if (sscanf(eq, "%lu", lu) == 1)
+ /* Detect non-numeric chars. */
+ char c;
+ if (sscanf(eq, "%lu%c", lu, &c) == 1)
return URJ_STATUS_OK;
}
else
@@ -198,7 +200,9 @@ parse_param_lu(const char *eq, long unsi
return URJ_STATUS_OK;
}

- urj_error_set (URJ_ERROR_SYNTAX, "need unsigned int, not '%s'", eq);
+ urj_error_set (URJ_ERROR_SYNTAX,
+ "%s: could not parse number (hex values start with 0x)",
+ eq);
return URJ_STATUS_FAIL;
}

-mike