Re: [PATCH] [media] tw5864: handle unknown video std gracefully

[Andrey Utkin]

On Tue, Feb 28, 2017 at 09:20:53AM +0100, Arnd Bergmann wrote:
> On Tue, Feb 28, 2017 at 2:08 AM, Andrey Utkin
> <andrey.utkin@xxxxxxxxxxxxxxxxxxx> wrote:
> > Retcode checking takes place everywhere, but currently it overwrites
> > supplied structs with potentially-uninitialized values. To make it
> > cleaner, it should be (e.g. tw5864_g_parm())
> >
> > ret = tw5864_frameinterval_get(input, &cp->timeperframe);
> > if (ret)
> >         return ret;
> > cp->timeperframe.numerator *= input->frame_interval;
> > cp->capturemode = 0;
> > cp->readbuffers = 2;
> > return 0;
> >
> > and not
> >
> > ret = tw5864_frameinterval_get(input, &cp->timeperframe);
> > cp->timeperframe.numerator *= input->frame_interval;
> > cp->capturemode = 0;
> > cp->readbuffers = 2;
> > return ret;
> >
> > That would resolve your concerns of uninitialized values propagation
> > without writing bogus values 1/1 in case of failure. I think I'd
> > personally prefer a called function to leave my data structs intact when
> > it fails.
> 
> That seems reasonable, I can try to come up with a new version that
> incorporates this change, but I haven't been able to avoid the warning
> without either removing the WARN() or adding an initialization.

I don't mind dropping WARN().

Thanks for your elaborate reply.