On Wed, 18 Jul 2018, Daniel Thompson wrote: > On Wed, Jul 18, 2018 at 02:08:53PM +0100, Lee Jones wrote: > > > > > > No, then we are back to the initial issue of num_steps > > > > > > potentially not > > > > > > being initialised. We really want both of_property_read_u32() to > > > > > > succeed AND num_steps to actually be set. > > > > > > > > > > I also think num_steps should be pre-initialised. > > > > > > Yes, I guess it definitely does not hurt. > > > > > > > > Then it will only be set if of_property_read_u32() succeeds. > > > > > > Yes, but we still need to check for both, the function not failing and > > > num_steps to actually be non zero. > > > > Why? You don't do anything differently if it fails. > > Only if you initialize num_steps... > > We should either initialize to zero and not worry about the return > code[1] or we check the return code and not worry about > initialization[2]. I don't think both are worthwhile. > > Whilst initialization can fix this specific instance we generally avoid > overusing it since it messes up static analysis and, in this instance, > distance from declaration to use is >25 lines, hence current patchset. > > > Daniel. > > > [1] https://lkml.org/lkml/2018/7/16/399 > [2] https://lkml.org/lkml/2018/7/16/1042 > > Or... > > We check the return code and leave number > > num_steps is uninitialized and stack allocated so it only has a valid > value if of_property_read_u32() succeeds. > > We can (and I originally did) fix the bug by initializing num_steps to 0 > but its quite some distance between declaration and use so I accepted > Marcel's counter proposal to check the return code instead.
Only checking the return value of of_property_read_u32() is also suitable. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
