[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bluetooth-dev] hci_inquiry(...) issues



hi there,

i have got a couple of remarks on inquiry mode (hci_inquiry(...) in
hci.c:1838):

a) error checking: range of parameters not checked in hci_inquiry(...)
   for validity.
b) application api: there is no way to receive all inquiry results
   after a hci_inquiry(...).
c) performance issue: inq_res in hci_inquiry(...) is frequently freed/
   allocated. instead, it could be reused.


a) hci_inquiry(u8 *lap, u8 inq_len, u8 num_resp) (in hci.c:1838)
   causes the bt-device to enter inquiry mode for the duration of n *
   1.28 secs, n = [ 1, ..., 48]. the range of n is not checked. values
   larger 48 result in "BT SYS: ERROR :COMMAND_STATUS: Invalid HCI
   Command Parameters".  pragmatic solution: add

   if( inq_len== 0) inq_len= 1;
   if( inq_len> 48) inq_len=48;

b) hci_inquiry(...) returnes inquiry results received during making
   the call to hci_inquiry(...). however it does not necessarily
   return all inquiry results received during inquiry mode (inquiry
   mode may last up to 62 seconds)! in my setup with 3 devices, one is
   frequently missed. there is no way to access inquiry
   results received from the hci layer after hci_inquiry(...) has
   returned. i would think, there need be a callback into application
   code called after receiving the INQUIRY_COMPLETE event. like:

// somewhere in hci.c:
// define a function variable
static void (*app_inquiry_complete_cb)(inquiry_results *res) = NULL;

// function to register application callback with.
// call it "axis" since its not in the bt-spec
axis_register_app_inquiry_complete_cb( void(*x)(inquiry_results *res) ){
  if( ! app_inquiry_complete_cb )
     app_inquiry_complete_cb=x;
  else
    // error: already registered!
}

// in process_event(...) in hci.c:
[...]
case INQUIRY_COMPLETE:
   printk("INQUIRY_COMPLETE\n");
   wake_up_interruptible(&inq_wq);
   if( app_inquiry_complete_cb ) app_inquiry_complete_cb( inq_res );
   break;

   however, then we would need a semaphore not to free inq_res
   (i.e. call hci_inquiry(...) again) before app_inquiry_complete_cb()
   returnes.

c) finally, in hci_inquiry(...) inq_res is always freed and
   re-allocated, even if the size of the buffer hasn't changed since
   the last call to hci_inquiry(...). a function-static variable for
   saving the num_resp parameter of the last call could help. btw, an
   num_resp of 0 means unlimited number of responses.

hci_inquiry(u8 *lap, u8 inq_len, u8 num_resp)
{
        s32 tmp;
        static last_num_resp = num_resp;

        if(num_resp==0)
           // error: unlimited number of responses not supported!
           [...] 

        D_CTRL("Sending inquiry()\n");

        [...]

        if(last_num_resp < num_resp) // need larger buffer
           if (!inq_res)
              kfree(inq_res);

        inq_res = (inquiry_results*) kmalloc(sizeof(inquiry_results)

        [...]   

ugh, this mail got longer than i thought.

cheers,
olli*


-- 
Oliver Kasten                                phone: +41/ 1/ 63-2 06 63
ETH-Zurich                                     fax: +41/ 1/ 63-2 16 59
Haldeneggsteig 4, IFW D48.1                  email: kasten@xxxxxxx.ch
CH-8092 Zuerich, Switzerland           http://www.inf.ethz.ch/~kasten/
-
To unsubscribe from this list: send the line "unsubscribe bluetooth-dev" in
the body of a message to majordomo@xxxxxxx.com