Reputation: 1574
I have written a C library function, but the return value seems to be incorrect, even though it is correct in the function.
Here is relevant code:
(In dcml_private.c) The offending function:
dcml_status _dcml_get_status(struct dcml_device *dev)
{
uint64_t data;
dcml_status ret;
int len;
libusb_bulk_transfer(dev->handle,
DCML_ENDPOINT | LIBUSB_ENDPOINT_IN,
(unsigned char *) &data,
DCML_REPORT_SZ, &len, RX_TIMEOUT);
printf("data = %ld\n", data);
if (len != DCML_REPORT_SZ)
printf("DCML: LIBUSB ERROR (%s)\n", libusb_error_name(len));
return STATUS_UNKNOWN;
ret = data & ~(1>>17);
return (ret);
}
The calling function:
void _dcml_cmd(dcml_context *ctx, dcml_cmd cmd,
dcml_status quit_cond, int dur)
{
struct timeval start;
struct timeval cur;
uint32_t stat;
(void)gettimeofday(&start, NULL);
(void)gettimeofday(&cur, NULL);
_dcml_send_cmd(ctx->active, cmd);
while(difftimeval(cur, start) < dur) {
sleep(POLL_PERIOD);
stat = _dcml_get_status(ctx->active);
printf("status (%d), quit_cond (%d)", stat, quit_cond);
if (stat == quit_cond)
break;
(void)gettimeofday(&cur, NULL);
}
_dcml_send_cmd(ctx->active, CMD_NONE);
}
As you see, I have print statements in my functions. In _dcml_cmd, typical output of that print statement would be
status (65535), quit_cond (2048)
Where _dcml_get_status prints:
data = 128
What this means is that the return value is correct IMMEDIATELY before exiting _dcml_get_status, but incorrect immediately after it is return to the calling function (and always has a value of 65535 here...)
It's probably helpful to know that "dmcl_status" is an enum. Switching the return type to uint16_t does not fix the issue. I thought it might be an overflow issue or something, but changing types, explicit casts and adding a mask line doesn't fix it.
Any thoughts?
Upvotes: 2
Views: 1756
Reputation: 22402
It's because you have a bad habit of not putting { } after your if statements ALWAYS
if (len != DCML_REPORT_SZ) {
printf("DCML: LIBUSB ERROR (%s)\n", libusb_error_name(len));
return STATUS_UNKNOWN;
}
also use fprintf(stderr,...
to dump out errors. Sending errors to printf is bad practice:
if (len != DCML_REPORT_SZ) {
fprintf(stderr, "DCML: LIBUSB ERROR (%s)\n", libusb_error_name(len));
return STATUS_UNKNOWN;
}
The calling function has the same issues around:
printf("status (%d), quit_cond (%d)", stat, quit_cond);
if (stat == quit_cond)
break;
Use:
fprintf(stderr, "status (%d), quit_cond (%d)", stat, quit_cond);
if (stat == quit_cond) {
break;
}
Yes it uses up an extra line but darn it at 3:00AM when you are debugging this by adding fprintfs all over the place it won't break your logic. :^)
Upvotes: 8