Hello Mr. Torokhov,
On Fri, May 13, 2016 at 8:34 PM, Aniroop Mathur
<aniroop.mathur@xxxxxxxxx> wrote:
> On Fri, May 13, 2016 at 4:01 AM, Dmitry Torokhov
> <dmitry.torokhov@xxxxxxxxx> wrote:
>> Hi Aniroop,
>>
>> On Fri, May 13, 2016 at 03:57:06AM +0530, Aniroop Mathur wrote:
>>> Hello Mr. Torokhov,
>>>
>>> Would you please help to update on the below patch?
>>> I have fixed the out of bound error which you mentioned last time.
>>> I have been really waiting for a long time. Please be kind to conclude it.
>>
>> I believe that this problem is a best theoretical and does not cause
>> issues with any of the drivers at present and therefore is not a
>> priority. Given that we already took quite a few iterations trying to
>> fix the issue without introducing worse regressions I'd like to leave
>> the code as is for now.
>>
>
> I agree that this part of code is not used by any driver so far but there
> are other factors to consider as well. There are many people in the
> world who develop concepts about kernel by reading the code as this
> is the best way to do so. If someone (especially beginners) read this
> part of code then he/she will learn wrong concept for SYN_REPORT
> event and "as more time passes more people will read this code
> and hence more will end up in a state of doubt whether it is correct to
> insert SYN_REPORT event in between a single packet or not?" This
> looks more dangerous to me.
> So to avoid that this code misguides someone now and in future, I
> believe it is very good reason to fix it. I believe that whether some
> driver uses it or not is a second concern and foremost concern is
> that code should follow protocol and should not mislead someone
> to develop wrong concepts.
>
> It happened many times that people needed more than 3 iterations
> to submit a patch so it looks normal to me that this patch too took 3
> iterations (especially when people are in the phase of learning kernel)
> and now the fix looks good to apply with no problems.
> I am sure you agree on the part that code review, code reading are
> best ways to learn concepts about kernel core and so kernel
> subsystem protocols should be given priority. Keeping this in mind,
> I request you again to kindly consider the patch for submission
> to fix this protocol part.
>
> Hope to hear positive reply from you. Have a good day!
>
How about above? Isn't this sufficient enough reason to fix this bug?
> Thanks,
> Aniroop Mathur
>> Thanks.
>>
>>>
>>> Thanks,
>>> Aniroop Mathur
>>>
>>> On Wed, May 11, 2016 at 7:40 PM, Aniroop Mathur
>>> <aniroop.mathur@xxxxxxxxx> wrote:
>>> > Hello Mr. Torokhov,
>>> >
>>> > Would you please provide update for this patch?
>>> > As you can notice it is really pending for a long time,
>>> > I'll be grateful if you could update and conclude it quickly.
>>> >
>>> > Thank you.
>>> >
>>> > BR,
>>> > Aniroop Mathur
>>> >
>>> >
>>> > On Wed, May 4, 2016 at 9:12 PM, Aniroop Mathur <a.mathur@xxxxxxxxxxx> wrote:
>>> >> As mentioned in documentation, SYN_REPORT should be used to separate two packets
>>> >> and should not be inserted in between a single packet as otherwise with multiple
>>> >> SYN_REPORT in a single packet, input reader would not be able to know when the
>>> >> packet ended really.
>>> >>
>>> >> Documentation snippet:
>>> >> * SYN_REPORT:
>>> >> - Used to synchronize and separate events into packets of input data changes
>>> >> occurring at the same moment in time. For example, motion of a mouse may set
>>> >> the REL_X and REL_Y values for one motion, then emit a SYN_REPORT. The next
>>> >> motion will emit more REL_X and REL_Y values and send another SYN_REPORT.
>>> >>
>>> >> Signed-off-by: Aniroop Mathur <a.mathur@xxxxxxxxxxx>
>>> >> ---
>>> >> drivers/input/input.c | 12 +++++++-----
>>> >> 1 file changed, 7 insertions(+), 5 deletions(-)
>>> >>
>>> >> diff --git a/drivers/input/input.c b/drivers/input/input.c
>>> >> index 8806059..5b0b1ae 100644
>>> >> --- a/drivers/input/input.c
>>> >> +++ b/drivers/input/input.c
>>> >> @@ -401,12 +401,14 @@ static void input_handle_event(struct input_dev *dev,
>>> >> if (dev->num_vals >= 2)
>>> >> input_pass_values(dev, dev->vals, dev->num_vals);
>>> >> dev->num_vals = 0;
>>> >> - } else if (dev->num_vals >= dev->max_vals - 2) {
>>> >> - dev->vals[dev->num_vals++] = input_value_sync;
>>> >> - input_pass_values(dev, dev->vals, dev->num_vals);
>>> >> - dev->num_vals = 0;
>>> >> + } else if (dev->num_vals >= dev->max_vals - 1) {
>>> >> + /* Pass all events except the newest event in order to
>>> >> + * not suppress the immediate EV_SYN/SYN_REPORT event.
>>> >> + */
>>> >> + input_pass_values(dev, dev->vals, dev->num_vals - 1);
>>> >> + dev->vals[0] = dev->vals[dev->num_vals - 1];
>>> >> + dev->num_vals = 1;
>>> >> }
>>> >> -
>>> >> }
>>> >>
>>> >> /**
>>> >> --
>>> >> 2.6.2
>>> >>
>>
>> --
>> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
