Hi!
> > +static inline void hci_h4p_handle_byte(struct hci_h4p_info *info, u8 byte)
>
> pretty big function to be inline
Called from just one place, so that should be ok.
> > +static void hci_h4p_rx_tasklet(unsigned long data)
> > +{
> []
> > + while (hci_h4p_inb(info, UART_LSR) & UART_LSR_DR) {
> []
> > + pr_debug("0x%.2x ", byte);
>
> pr_debug is prefixed by a newline if necessary
> and then <7>, one for each use.
>
> This will produce a lot of dmesg output lines
> (1 for each byte) and isn't in my opinion
> necessary/useful.
Ok, I just killed it.
> > + if (set != !!test_bit(H4P_ACTIVE_MODE, &info->pm_flags)) {
> > + bt_plat_data->set_pm_limits(info->dev, set);
> > + if (set)
> > + set_bit(H4P_ACTIVE_MODE, &info->pm_flags);
> > + else
> > + clear_bit(H4P_ACTIVE_MODE, &info->pm_flags);
> > + BT_DBG("Change pm constraints to: %s", sset);
>
> missing newline
Actually, it is the other way around. BT_DBG adds the newline. I'll
remove it from the rest.
> > + if (!hdev) {
> > + printk(KERN_WARNING "hci_h4p: Frame for unknown device\n");
> > + return -ENODEV;
> > + }
>
> Is this possible?
Probably not, removed.
> > + if (ret != 6)
> > + return -EINVAL;
> > +
> > + for (i = 0; i < 6; i++)
> > + info->bd_addr[i] = bdaddr[i] & 0xff;
>
> This could also return -EINVAL if bdaddr[i] > 0xff
Why not.
> > +static int hci_h4p_bcm_set_bdaddr(struct hci_h4p_info *info, struct
> > sk_buff *skb)
> > +{
> > + int i;
> > + static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
> > + int not_valid;
> > +
> > + not_valid = 1;
> > + for (i = 0; i < 6; i++) {
> > + if (info->bd_addr[i] != 0x00) {
> > + not_valid = 0;
> > + break;
> > + }
> > + }
>
> This seems wrong as addresses can have valid 0 bytes.
> Perhaps use:
>
> if (!is_valid_ether_addr(info->bd_addr))
>
I am not sure bluetooth rules are same as ethernet. And notice that it
only errors out on 00:00:00:00:00:00 which seems like invalid address
to me.
I fixed the other ones.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
