10-29-2009 12:00 PM - edited 10-29-2009 12:00 PM
I have the feeling I'^m doing something really silly here but I can't figure it out.
When I run this piece of code, I should theoretically get back a valid pointer when the correct Pid and Vid for a USB device is found. At the moment, it works ONLY if I run it with the correct Vid and Pid vlaues first (value returned is NULL) and then call it with a DIFFERENT Vid and Pid.
Is there a timing problem with the usb_close or something?
Code below:
struct usb_dev_handle *Opendev(int vid, int pid, int Interface)
{
struct usb_bus *bus;
struct usb_device *dev;
char *message;
message = StrDup(" ");
InitUSB();
usb_find_devices();
for (bus=usb_get_busses();bus;bus=bus->next)
{
for (dev=bus->devices;dev;dev=dev->next)
{
if (dev->descriptor.idProduct == pid)
{
if (dev->descriptor.idVendor == vid)
{
sprintf(message, " idProduct %04x, pid %04x", dev->descriptor.idProduct, pid);
MessagePopup(message, message);
// if (current)
// {
// usb_close(current);
// }
current = usb_open(dev);
if (current)
{
return current;
}
}
}
}
}
return NULL;
}
As you can see, I've tried running it without the close function but the same thing is happening.
Help?
Shane.
PS "current" is a global variable defined within the DLL
Solved! Go to Solution.
10-29-2009 02:09 PM
Can't help with the USB specifics - I've never used these functions, but you've got a small memory leak. You need to free "message" before you leave the routine. Have you shown us the full code? The "Interface" parameter doesn't seem to be used.
When you say it works are you referring to the expected popup message appearing, or a valid pointer being generated?
Mike
10-29-2009 02:16 PM
Hmm, memory leak? Doesn't the memory space get reused on multiple calls? I'm a LV veteran, a bit rusty on good 'ol C.
I know Interface isn't used, it's going to be used once the rest is working. One step at a time.
When I say it works, I mean that -after calling the routine with the correct Pid and Vid values- running it again with DIFFERENT values (so that the If statements are false) I then get the correct value returned. Otherwise I get zero (NULL).
The popup appears every time as expected but the value returned seems to be old. It would seem like I have a race condition somewhere, but I don't know where.
When the function returns a valid pointer, Other USB functions which accept this pointer as an argument work fine. When it returns NULL they don't (obviously).
On a side-note... How can I output the address of a pointer in text-form in a Messagebox? That's something I've wanted to do for a while but can't figure out how to format a pointer to something into a numerical text.....
Shane.
10-30-2009 03:22 AM - edited 10-30-2009 03:31 AM
OK, I have implemented a different message popup. I can now view the address of the pointer after opening the device (The value which SHOULD be returned).
The funny part is that when I do this, the message box shows the correct value (which is the same as returned later when calling the function with wrong pid and vid as explained before), but the function still returns zero.
Could it be that I'm getting an unhandled error or something which is preventing the function from returning the correct value?
I really don't understand what's going on.
struct usb_dev_handle *Opendev(int vid, int pid, int Interface)
{
struct usb_bus *bus;
struct usb_device *dev;
char *message;
message = StrDup(" ");
InitUSB();
usb_find_devices();
for (bus=usb_get_busses();bus;bus=bus->next)
{
for (dev=bus->devices;dev;dev=dev->next)
{
if (dev->descriptor.idProduct == pid)
{
if (dev->descriptor.idVendor == vid)
{
current = usb_open(dev);
sprintf(message, "Pointer = %08x.", current);
MessagePopup(message, message);
return current;
}
}
}
}
return current;
}
So the return statement two lines below the messagebox showing the address of the poiinter returns a different value than the messagebox.
Can anyone help me?
Shane.
Ps A "return" statement in the middle of the code is legal, right
PPS moving the return to the end of the function makes no difference. I'm confused.
10-30-2009 04:48 AM
If your (latest) code fails to find a pid/vid match, it will return the existing value of current, which presumably is a valid pointer from a previous successful search. This could be a source of confusion. Why did you change it from your earlier code which correctly returns NULL under these circumstances?
It is perfectly legal to have a return statement in the middle of code. It is a practice often frowned upon, because it can complicate arrangements to free memory or close handles which may have been opened earlier in the function.
(Speaking of freeing memory, instead of:
char *message;
message = StrDup(" ");
...
free (message);
you might consider that:
char message [100];
is a bit simpler and functionally identical in your usage.)
I am not familiar with the USB library you are using, but it seems a little suspicious that you are calling InitUSB() every time you enter your function, without a corresponding CloseUSB()? I would expect such functions to be called only once, at the start/end of the main program code.
JR
10-30-2009 04:56 AM
I changed the final return, true. It was simply an attempt to try to get a working scenario. It didn't help.
I call the Init every time, but I set a global after the first time so the rest of the time it's not actually doing anything.
I have this in every exported function because I want to unburden the user from having to call the Init function himself.
Close is called explicitly from outside the DLL when the entire processing is finished.
Re: Char[100]. Ah, that's where I thought the memory would be re-used. I'll do that.
To the actual problem: I'm aware that the valid value returned when the pid and vid are NOT matched is the result of a previous iteration. I just don't understand why the iterations where a match IS found will NOT return this value, even though the messagebox shows that the variable "current" has the correct value.
This inability to return the correct value is the source of my confusion.
Shane.
10-30-2009 05:18 AM
Intaris wrote:
I call the Init every time, but I set a global after the first time so the rest of the time it's not actually doing anything.
I have this in every exported function because I want to unburden the user from having to call the Init function himself.
Ah yes, I remember this from a previous post on that topic, now.
So you're saying that the message box shows the correct value, but the return value is not the same? How are you inspecting the return value - with the debugger? Can you show us the code for the call to this function?
The only time I have seen a similar problem was when the function return mechanism effectively cast the value to something slightly different, so that the caller received a modified version of the intended value. (Something to do with unsigned shorts in an int function, IIRC.) It goes without saying that your current variable is of type struct usb_dev_handle * ?
JR
10-30-2009 05:35 AM
I'm calling the DLL from LabVIEW, this is how I'm viewing the return value.
I use the return value to call other LibUSB DLL functions without my Wrapper DLL. This tells me fairly well if the returned pointer value is correct or not. If it's not correct, the LibUSB DLL will throw an error.
The definition of the variable "current" is indeed "usb_dev_handle *current;"
Below is a pic of my DLL call in LV. Nothing really special there. Of course I'm treating the returned pointer as an I32, not a pointer (Since LV doesn't allow those). You reckon the problem could have to do with that?
Would that explain why it only gets returned on the second call after assignment? Seems like pretty wierd behaviour to me.
The second DLL call is to the actual LibUSB library to do a simple device descriptor request (supported by all USB devices). If that returns sensible data, everything's fine.
Shane.
10-30-2009 05:56 AM
WOW! Your C skills might be a bit rusty, but my LabView ones simply do not exist! You might as well have posted a Jackson Pollock painting, for all the sense your diagram makes to me
One line of thought springs to mind, though: did you set the calling convention to match the one used when you built the dll? Mismatched conventions have been known to produce very odd results.
I can't tell what LabView does with the return value - I wouldn't expect a C program (in a 32 bit environment, anyway) to have problems with the intrinsic cast from a pointer to an int, but, to be on the safe side, I would try using an unsigned int or even an unsigned _int64 (or whatever the LabView equivalents are) in the LabView panel for the prototype, just to eliminate this as a possible problem.
So how do you know what the return value is? Presumably LabView has similar debug/variable inspection facilities to CVI? What value does it show, compared to the value printed in the message box?
JR
10-30-2009 06:13 AM
Did you just say my LV code is a load of Pollocks?
Changing the calling convention is pretty much the first thing I try to change if things don't work as expected. I've exhausted that avenue unfortunately.
I'll try a U32, but I doubt it'll change anything somehow.
In LV, the value is exactly the same as in the DLL messagebox (when it's returned). On the first call it's always zero. NULL. Maybe it's a LV allocation issue.....
One thing I CAN telly ou about LabVIEW is that debugging is a lot easier than in CVI. At least for someone with 15 years LV experience and about 15 days CVI experience.....
Shane.