Time for a code audit?

Dan Carpenter dan.carpenter at oracle.com
Mon May 2 09:52:57 UTC 2016


Reviewing drivers/staging/unisys/visorhba/visorhba_main.c

visor_thread_start()
I'm nervous about the error handling here.  Is setting ->id to zero
really sufficient to prevent dereferencing ->task?

Still a lot of return -1;
grep "return -1;" drivers/staging/unisys/ -R

del_scsipending_ent()
flip the if condition around so it's error handling instead of success
handling.  Remove the "sent = NULL" initializer.

drivers/staging/unisys/visorhba/visorhba_main.c
   301          /* specify the event that has to be triggered when this */
   302          /* cmd is complete */
   303          cmdrsp->scsitaskmgmt.notify_handle = (u64)¬ifyevent;
   304          cmdrsp->scsitaskmgmt.notifyresult_handle = (u64)¬ifyresult;

This casting an int pointer to u64 pointer seems bad.

I must be blind.  How does queue_disk_add_remove() work?  Where do we
initialize dar_work_queue?

process_disk_notify()
switch success handling to error handling.

visorhba_probe().
We really can only probe one device right?  Because VISORHBA_OPEN_MAX is
1.  So what happens if we try to probe two?  I don't immediately see how
that is handled.

visorhba_init()
return a literal zero on success.

Looks pretty good generally to mee.

regards,
dan carpenter


More information about the devel mailing list