Char signedness bug in usbGenericSetInterrupt() [fixed]

General discussions about V-USB, our firmware-only implementation of a low speed USB device on Atmel's AVR microcontrollers
Post Reply
blargg
Rank 3
Rank 3
Posts: 102
Joined: Thu Nov 14, 2013 10:01 pm

Char signedness bug in usbGenericSetInterrupt() [fixed]

Post by blargg » Thu Nov 14, 2013 10:03 pm

Code: Select all

static void usbGenericSetInterrupt(uchar *data, uchar len, usbTxStatus_t *txStatus)
{
uchar   *p;
char    i;

[...]
    i = len;
    do{                         /* if len == 0, we still copy 1 byte, but that's no problem */
        *p++ = *data++;
    }while(--i > 0);            /* loop control at the end is 2 bytes shorter than at beginning */
[...]


It sure seems it's assuming that char is signed. It's common to use -funsigned-char to avoid unnecessary sign-extension, which would apparently break the above code. There is an schar in usbdrv.h:

Code: Select all

#ifndef schar
#define schar   signed char
#endif


so it doesn't seem they want to rely on char being signed.
Last edited by blargg on Thu Nov 14, 2013 10:28 pm, edited 2 times in total.

christian
Objective Development
Objective Development
Posts: 1442
Joined: Thu Nov 09, 2006 11:46 am

Re: V-USB assumes that char is signed?!?

Post by christian » Thu Nov 14, 2013 10:16 pm

Please check out the current head on the master branch on github:

https://github.com/obdev/v-usb

blargg
Rank 3
Rank 3
Posts: 102
Joined: Thu Nov 14, 2013 10:01 pm

Re: V-USB assumes that char is signed?!?

Post by blargg » Thu Nov 14, 2013 10:24 pm

OK, so it does not assume that char is signed, and what I found today was a bug and happened to be fixed two days ago. Hah.

christian
Objective Development
Objective Development
Posts: 1442
Joined: Thu Nov 09, 2006 11:46 am

Re: Char signedness bug in usbGenericSetInterrupt() [fixed]

Post by christian » Thu Nov 14, 2013 10:34 pm

It has been fixed months ago, but I forgot to commit and push it. The commit message is also a bit confusing since I did not remember what that particular fix was good for.

Post Reply