Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread Dan Carpenter
On Thu, Oct 19, 2017 at 04:16:37PM +0200, Michal Suchánek wrote: > On Thu, 19 Oct 2017 16:36:46 +0300 > Dan Carpenter wrote: > > > On Thu, Oct 19, 2017 at 01:56:32PM +0200, Michal Suchánek wrote: > > > > > > I think a single cleanup section is better than many labels

Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread Dan Carpenter
tpm_ibmvtpm_probe() is an example of poor names. It has the generic ones like "cleanup" which don't say *what* is cleaned and the come-from ones like "init_irq_cleanup" which don't say anything useful at all: 647 rc = request_irq(vio_dev->irq, ibmvtpm_interrupt, 0, 648

Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread Michal Suchánek
On Thu, 19 Oct 2017 16:36:46 +0300 Dan Carpenter wrote: > On Thu, Oct 19, 2017 at 01:56:32PM +0200, Michal Suchánek wrote: > > > > I think a single cleanup section is better than many labels that > > just avoid a single null check. > > > > I am not a big advocate of

Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread Dan Carpenter
On Thu, Oct 19, 2017 at 01:56:32PM +0200, Michal Suchánek wrote: > > I think a single cleanup section is better than many labels that just > avoid a single null check. > I am not a big advocate of churn, but one err style error handling is really bug prone. I'm dealing with static analysis so

Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread Michal Suchánek
On Thu, 19 Oct 2017 14:36:09 +0200 SF Markus Elfring wrote: > >> @@ -683,13 +683,10 @@ static int tpm_ibmvtpm_probe(struct vio_dev > >> *vio_dev, reg_crq_cleanup: > >>dma_unmap_single(dev, ibmvtpm->crq_dma_handle, > >> CRQ_RES_BUF_SIZE, DMA_BIDIRECTIONAL); > >>

Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread SF Markus Elfring
>> @@ -683,13 +683,10 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, >> reg_crq_cleanup: >> dma_unmap_single(dev, ibmvtpm->crq_dma_handle, >> CRQ_RES_BUF_SIZE, DMA_BIDIRECTIONAL); >> -cleanup: >> -if (ibmvtpm) { >> -if (crq_q->crq_addr) >> -

Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-19 Thread Michal Suchánek
Hello, On Mon, 16 Oct 2017 19:34:56 +0200 SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 16 Oct 2017 19:00:34 +0200 > > Two pointer checks could be repeated by the tpm_ibmvtpm_probe() > function during error handling

[PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

2017-10-16 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 16 Oct 2017 19:00:34 +0200 Two pointer checks could be repeated by the tpm_ibmvtpm_probe() function during error handling even if the relevant properties can be determined for the involved variables before by source code analysis. *