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.
