On Sat, Jan 23, 2016 at 05:51:50AM +0000, Singhal, Maneesh wrote:
> Thanks for your time. My replies inlined...
>
[...]
> > > + }
> > > +
> >
> > You don't do any cleanup work at
> > ctd_scsi_response_sanity_check_complete. You
> > could just reutrn 0 here as well.
> [MS>] Just avoiding multiple exit points from the function
Please have a look at Documentation/CodingStyle Chapter 7: Centralized exiting
of functions. Especially this part:
<quote>
The goto statement comes in handy when a function exits from multiple
locations and some common work such as cleanup has to be done. If there is no
cleanup needed then just return directly.
</quote>
and the example below that section.
> >
> > > +
> > > + ctd_dprintk_crit(
[...]
> > > + if (request && request->io_timeout < EMCCTD_MAX_RETRY) {
> >
> > The following block is a bit long and therefore it's hard to spot an
> > eventual
> > error of handling the io_mgmt_lock. Can't it be factored out in a helper
> > function?
> >
> [MS>] I know its little longer, but actually fits logically together... Will
> see if it can be factored. Not sure though.
Yes please. It's not uber important but short functions and paths are
generally favoured when debugging and reviewing code.
Thanks,
Johannes
--
Johannes Thumshirn Storage
jthumsh...@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
