Skip to contentSkip to conversation

Bluetooth: hciuart: Add support QCA chipset for UART

DISCUSSION Dan Carpenter 2 participants 4 messages Aug 13, 2015 – Aug 14, 2015 Bluetooth drivers

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Ben Young Tae Kim,

The patch 0ff252c1976d: "Bluetooth: hciuart: Add support QCA chipset for UART" from Aug 10, 2015, leads to the following static checker warning:

	drivers/bluetooth/hci_qca.c:485 qca_debugfs_init()
	warn: passing casted pointer '&qca->tx_vote' to 'debugfs_create_bool()' 1 vs 32.

drivers/bluetooth/hci_qca.c
482 &qca->ibs_recv_wakes);
483 debugfs_create_u64("ibs_recv_wake_acks", mode, ibs_dir,
484 &qca->ibs_recv_wacks);
485 debugfs_create_bool("tx_vote", mode, ibs_dir, (u32 *)&qca->tx_vote);

debugfs_create_bool() is stupid, because the name says bool but it writes a u32. I am not sure why this is. Maybe because the sizeof _Bool is not defined in C? Anyway, this will not work and will also corrupt memory.

   486          debugfs_create_u64("tx_votes_on", mode, ibs_dir, &qca->tx_votes_on);
   487          debugfs_create_u64("tx_votes_off", mode, ibs_dir, &qca->tx_votes_off);
   488          debugfs_create_bool("rx_vote", mode, ibs_dir, (u32 *)&qca->rx_vote);
                               ^^^^

Same.

   489          debugfs_create_u64("rx_votes_on", mode, ibs_dir, &qca->rx_votes_on);
   490          debugfs_create_u64("rx_votes_off", mode, ibs_dir, &qca->rx_votes_off);

regards,
dan carpenter

#1 lore raw

Hi Dan,

Quoted 15 lines · Dan Carpenter

Hello Ben Young Tae Kim,

The patch 0ff252c1976d: "Bluetooth: hciuart: Add support QCA chipset for UART" from Aug 10, 2015, leads to the following static checker warning:

	drivers/bluetooth/hci_qca.c:485 qca_debugfs_init()
	warn: passing casted pointer '&qca->tx_vote' to 'debugfs_create_bool()' 1 vs 32.

drivers/bluetooth/hci_qca.c

    482                             &qca->ibs_recv_wakes);
    483          debugfs_create_u64("ibs_recv_wake_acks", mode, ibs_dir,
    484                             &qca->ibs_recv_wacks);
    485          debugfs_create_bool("tx_vote", mode, ibs_dir, (u32 *)&qca->tx_vote);
Quoted 4 lines

debugfs_create_bool() is stupid, because the name says bool but it writes a u32. I am not sure why this is. Maybe because the sizeof _Bool is not defined in C? Anyway, this will not work and will also corrupt memory.

I agree with you. The reason why I do the cast as u32 type pointer is debugfs_create_bool() function is required to put value as u32 *. But it will cause memory corruption to read that because it is actual 1 byte. Thanks for your report. I'll submit the patch to address it soon.

Quoted 7 lines
    486          debugfs_create_u64("tx_votes_on", mode, ibs_dir, &qca->tx_votes_on);
    487          debugfs_create_u64("tx_votes_off", mode, ibs_dir, &qca->tx_votes_off);
    488          debugfs_create_bool("rx_vote", mode, ibs_dir, (u32 *)&qca->rx_vote);
                                ^^^^

Same.

Quoted 7 lines
    489          debugfs_create_u64("rx_votes_on", mode, ibs_dir, &qca->rx_votes_on);
    490          debugfs_create_u64("rx_votes_off", mode, ibs_dir, &qca->rx_votes_off);

regards,
dan carpenter

Thanks
-- Ben Kim

#2 lore raw

Hi Dan,

Quoted 9 lines · Ben Young Tae Kim

Hi Dan,

Quoted 6 lines · Dan Carpenter

Hello Ben Young Tae Kim,

The patch 0ff252c1976d: "Bluetooth: hciuart: Add support QCA chipset for UART" from Aug 10, 2015, leads to the following static checker warning:

BTW, which tool are you using to detect static warning? Can you recommend me some tool so that I could prevent same error to the upcoming patches?

Thanks
-- Ben Kim

author
#3 lore raw
Quoted 4 lines · Ben Young Tae Kim

BTW, which tool are you using to detect static warning? Can you recommend me some tool so that I could prevent same error to the upcoming patches?

This test is a Smatch test but it has too many false positives to publish. Many of them can probably be silenced... I will see what I can do.

regards,
dan carpenter