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