On Sat, Nov 29, 2014 at 10:04 PM, Nicholas Krause <xerofoify@xxxxxxxxx> wrote: > Fixs issue with setting the variable value of val in the functionm,lg2161_set_output_interface > and not sanity checking if the value has been correctly set with the correct value of the struct > state of type lg216x_state passed to the function by the calling code. > > Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx> > --- > drivers/media/dvb-frontends/lg2160.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/dvb-frontends/lg2160.c b/drivers/media/dvb-frontends/lg2160.c > index 5fd14f8..9aec63a 100644 > --- a/drivers/media/dvb-frontends/lg2160.c > +++ b/drivers/media/dvb-frontends/lg2160.c > @@ -459,7 +459,9 @@ static int lg2161_set_output_interface(struct lg216x_state *state) > goto fail; > > val &= ~0x07; > - val |= state->cfg->output_if; /* FIXME: needs sanity check */ > + val |= state->cfg->output_if; > + if (lg_fail(val)) > + goto fail; > > ret = lg216x_write_reg(state, 0x0014, val); > lg_fail(ret); > -- > 2.1.0 > NACK. Thank you for the patch, Nicholas, but the change that you propose won't actually help here -- val is a u8, and the lg_fail() macro tests for negative values. val can never be negative, and that's why I didn't add the "lg_fail()" test myself when I wrote this function. What's actually happening in this function is that the value is first set when reading register 0014, we take that value, unset the three lowest bits, then we raise the bits in state->cfg->output_if, and write it back to the same register. In fact, the FIXME comment should be removed -- this is not a bug anymore and the sanity check is built-in. That FIXME comment is a remnant of an older version of this function :-P If you want to send a patch that just removes the FIXME comment, that would be accepted, but this patch should not be merged. Cheers, Michael Ira Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
