* Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method
2011-09-15 19:40 [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method Daniel Kurtz
@ 2011-09-16 0:52 ` Joonyoung Shim
2011-09-16 5:10 ` Daniel Kurtz
2011-09-20 12:57 ` Iiro Valkonen
2011-10-06 9:59 ` Nick Dyer
2011-10-19 7:40 ` Daniel Kurtz
2 siblings, 2 replies; 7+ messages in thread
From: Joonyoung Shim @ 2011-09-16 0:52 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Dmitry Torokhov, Iiro Valkonen, Henrik Rydberg, Yufeng Shen,
linux-input, linux-kernel
Hi,
On 2011-09-16 오전 4:40, Daniel Kurtz wrote:
> Sysfs attribute show methods are always passed a buffer of length
> PAGE_SIZE. To keep from overwriting this buffer and causing havoc, use
> snprintf() to guarantee we never write more than the buffer can hold.
>
> In addition, at least for my touchscreen, the number and size of objects
> was far too big to fit in a single 4K page. Therefore, this patch also
> trims some redundant framing text to leave more room for actual data.
>
> Signed-off-by: Daniel Kurtz<djkurtz@chromium.org>
This patch is working but original problem is object size is increasing
as atmel touchscreen chip updates and we should consider also the
instances of object.
How about splitting attributes to each object?
Iiro, do you have any update plans about atmel touchscreen driver?
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 21 +++++++++++++++------
> 1 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index f5d6685..a596c27 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -910,12 +910,17 @@ static ssize_t mxt_object_show(struct device *dev,
> for (i = 0; i< data->info.object_num; i++) {
> object = data->object_table + i;
>
> - count += sprintf(buf + count,
> - "Object Table Element %d(Type %d)\n",
> + count += snprintf(buf + count, PAGE_SIZE - count,
> + "Object[%d] (Type %d)\n",
> i + 1, object->type);
> + if (count>= PAGE_SIZE)
> + return PAGE_SIZE - 1;
>
> if (!mxt_object_readable(object->type)) {
> - count += sprintf(buf + count, "\n");
> + count += snprintf(buf + count, PAGE_SIZE - count,
> + "\n");
> + if (count>= PAGE_SIZE)
> + return PAGE_SIZE - 1;
> continue;
> }
>
> @@ -925,11 +930,15 @@ static ssize_t mxt_object_show(struct device *dev,
> if (error)
> return error;
>
> - count += sprintf(buf + count,
> - " Byte %d: 0x%x (%d)\n", j, val, val);
> + count += snprintf(buf + count, PAGE_SIZE - count,
> + "\t[%2d]: %02x (%d)\n", j, val, val);
> + if (count>= PAGE_SIZE)
> + return PAGE_SIZE - 1;
> }
>
> - count += sprintf(buf + count, "\n");
> + count += snprintf(buf + count, PAGE_SIZE - count, "\n");
> + if (count>= PAGE_SIZE)
> + return PAGE_SIZE - 1;
> }
>
> return count;
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method
2011-09-16 0:52 ` Joonyoung Shim
@ 2011-09-16 5:10 ` Daniel Kurtz
2011-09-20 12:57 ` Iiro Valkonen
1 sibling, 0 replies; 7+ messages in thread
From: Daniel Kurtz @ 2011-09-16 5:10 UTC (permalink / raw)
To: Joonyoung Shim
Cc: Dmitry Torokhov, Iiro Valkonen, Henrik Rydberg, Yufeng Shen,
linux-input, linux-kernel
Hi Joonyoung,
Thanks for replying.
On Fri, Sep 16, 2011 at 8:52 AM, Joonyoung Shim <jy0922.shim@samsung.com> wrote:
> Hi,
>
> On 2011-09-16 오전 4:40, Daniel Kurtz wrote:
>>
>> Sysfs attribute show methods are always passed a buffer of length
>> PAGE_SIZE. To keep from overwriting this buffer and causing havoc, use
>> snprintf() to guarantee we never write more than the buffer can hold.
>>
>> In addition, at least for my touchscreen, the number and size of objects
>> was far too big to fit in a single 4K page. Therefore, this patch also
>> trims some redundant framing text to leave more room for actual data.
>>
>> Signed-off-by: Daniel Kurtz<djkurtz@chromium.org>
>
> This patch is working but original problem is object size is increasing
> as atmel touchscreen chip updates and we should consider also the
> instances of object.
"This patch is working" = Acked-By ?
> How about splitting attributes to each object?
I agree. Can we do that in another patch (with instance printing)?
> Iiro, do you have any update plans about atmel touchscreen driver?
I have several more patches coming for this driver, mostly to speed up
startup time, cleanup the MT-B implementation, and optimize interrupt
handling.
Thanks,
Daniel
>> ---
>> drivers/input/touchscreen/atmel_mxt_ts.c | 21 +++++++++++++++------
>> 1 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
>> b/drivers/input/touchscreen/atmel_mxt_ts.c
>> index f5d6685..a596c27 100644
>> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
>> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
>> @@ -910,12 +910,17 @@ static ssize_t mxt_object_show(struct device *dev,
>> for (i = 0; i< data->info.object_num; i++) {
>> object = data->object_table + i;
>>
>> - count += sprintf(buf + count,
>> - "Object Table Element %d(Type %d)\n",
>> + count += snprintf(buf + count, PAGE_SIZE - count,
>> + "Object[%d] (Type %d)\n",
>> i + 1, object->type);
>> + if (count>= PAGE_SIZE)
>> + return PAGE_SIZE - 1;
>>
>> if (!mxt_object_readable(object->type)) {
>> - count += sprintf(buf + count, "\n");
>> + count += snprintf(buf + count, PAGE_SIZE - count,
>> + "\n");
>> + if (count>= PAGE_SIZE)
>> + return PAGE_SIZE - 1;
>> continue;
>> }
>>
>> @@ -925,11 +930,15 @@ static ssize_t mxt_object_show(struct device *dev,
>> if (error)
>> return error;
>>
>> - count += sprintf(buf + count,
>> - " Byte %d: 0x%x (%d)\n", j, val,
>> val);
>> + count += snprintf(buf + count, PAGE_SIZE - count,
>> + "\t[%2d]: %02x (%d)\n", j, val,
>> val);
>> + if (count>= PAGE_SIZE)
>> + return PAGE_SIZE - 1;
>> }
>>
>> - count += sprintf(buf + count, "\n");
>> + count += snprintf(buf + count, PAGE_SIZE - count, "\n");
>> + if (count>= PAGE_SIZE)
>> + return PAGE_SIZE - 1;
>> }
>>
>> return count;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method
2011-09-16 0:52 ` Joonyoung Shim
2011-09-16 5:10 ` Daniel Kurtz
@ 2011-09-20 12:57 ` Iiro Valkonen
1 sibling, 0 replies; 7+ messages in thread
From: Iiro Valkonen @ 2011-09-20 12:57 UTC (permalink / raw)
To: Joonyoung Shim
Cc: Daniel Kurtz, Dmitry Torokhov, Henrik Rydberg, Yufeng Shen,
linux-input, linux-kernel
Hi,
On 09/16/2011 03:52 AM, Joonyoung Shim wrote:
>
> Iiro, do you have any update plans about atmel touchscreen driver?
>
yes, there are plans to update the driver. While I have been on a longish
vacation, the development work has continued in UK and new patches can be
expected in the near future.
Regards,
Iiro
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method
2011-09-15 19:40 [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method Daniel Kurtz
2011-09-16 0:52 ` Joonyoung Shim
@ 2011-10-06 9:59 ` Nick Dyer
2011-10-19 7:40 ` Daniel Kurtz
2 siblings, 0 replies; 7+ messages in thread
From: Nick Dyer @ 2011-10-06 9:59 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Joonyoung Shim, Dmitry Torokhov, Iiro Valkonen, Henrik Rydberg,
Yufeng Shen, linux-input, linux-kernel
Hi Daniel-
Just to introduce myself, I'm working for Atmel in UK on the
atmel_mxt_ts driver.
Daniel Kurtz wrote:
> Sysfs attribute show methods are always passed a buffer of length
> PAGE_SIZE. To keep from overwriting this buffer and causing havoc,
> use snprintf() to guarantee we never write more than the buffer can
> hold.
>
> In addition, at least for my touchscreen, the number and size of
> objects was far too big to fit in a single 4K page. Therefore, this
> patch also trims some redundant framing text to leave more room for
> actual data.
Yes, I'd noticed this problem as well. I was planning to address it by
splitting up the file, but your patch looks like a good solution for now.
I've tested it successfully on several chips.
Acked-by: Nick Dyer <nick.dyer@itdev.co.uk>
--
Nick Dyer
Software Engineer, ITDev Ltd
Hardware and Software Development Consultancy
Email: nick.dyer@itdev.co.uk
Website: http://www.itdev.co.uk
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method
2011-09-15 19:40 [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method Daniel Kurtz
2011-09-16 0:52 ` Joonyoung Shim
2011-10-06 9:59 ` Nick Dyer
@ 2011-10-19 7:40 ` Daniel Kurtz
2011-10-19 17:23 ` Dmitry Torokhov
2 siblings, 1 reply; 7+ messages in thread
From: Daniel Kurtz @ 2011-10-19 7:40 UTC (permalink / raw)
To: Dmitry Torokhov, Nick Dyer
Cc: Iiro Valkonen, Henrik Rydberg, Yufeng Shen, linux-input,
linux-kernel, Daniel Kurtz
Hi Dmitry,
I fear the following patch, and its 'Acked-by' from Nick Dyer, may
have gotten lost:
On Fri, Sep 16, 2011 at 3:40 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>
> Sysfs attribute show methods are always passed a buffer of length
> PAGE_SIZE. To keep from overwriting this buffer and causing havoc, use
> snprintf() to guarantee we never write more than the buffer can hold.
>
> In addition, at least for my touchscreen, the number and size of objects
> was far too big to fit in a single 4K page. Therefore, this patch also
> trims some redundant framing text to leave more room for actual data.
>
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 21 +++++++++++++++------
> 1 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index f5d6685..a596c27 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -910,12 +910,17 @@ static ssize_t mxt_object_show(struct device *dev,
> for (i = 0; i < data->info.object_num; i++) {
> object = data->object_table + i;
>
> - count += sprintf(buf + count,
> - "Object Table Element %d(Type %d)\n",
> + count += snprintf(buf + count, PAGE_SIZE - count,
> + "Object[%d] (Type %d)\n",
> i + 1, object->type);
> + if (count >= PAGE_SIZE)
> + return PAGE_SIZE - 1;
>
> if (!mxt_object_readable(object->type)) {
> - count += sprintf(buf + count, "\n");
> + count += snprintf(buf + count, PAGE_SIZE - count,
> + "\n");
> + if (count >= PAGE_SIZE)
> + return PAGE_SIZE - 1;
> continue;
> }
>
> @@ -925,11 +930,15 @@ static ssize_t mxt_object_show(struct device *dev,
> if (error)
> return error;
>
> - count += sprintf(buf + count,
> - " Byte %d: 0x%x (%d)\n", j, val, val);
> + count += snprintf(buf + count, PAGE_SIZE - count,
> + "\t[%2d]: %02x (%d)\n", j, val, val);
> + if (count >= PAGE_SIZE)
> + return PAGE_SIZE - 1;
> }
>
> - count += sprintf(buf + count, "\n");
> + count += snprintf(buf + count, PAGE_SIZE - count, "\n");
> + if (count >= PAGE_SIZE)
> + return PAGE_SIZE - 1;
> }
>
> return count;
> --
> 1.7.3.1
>
On Thu, Oct 6, 2011 at 5:59 PM, Nick Dyer <nick.dyer@itdev.co.uk> wrote:
> Hi Daniel-
>
> Just to introduce myself, I'm working for Atmel in UK on the atmel_mxt_ts
> driver.
>
> Daniel Kurtz wrote:
>> Sysfs attribute show methods are always passed a buffer of length
>> PAGE_SIZE. To keep from overwriting this buffer and causing havoc,
>> use snprintf() to guarantee we never write more than the buffer can
>> hold.
>>
>> In addition, at least for my touchscreen, the number and size of
>> objects was far too big to fit in a single 4K page. Therefore, this
>> patch also trims some redundant framing text to leave more room for
>> actual data.
>
> Yes, I'd noticed this problem as well. I was planning to address it by
> splitting up the file, but your patch looks like a good solution for now.
>
> I've tested it successfully on several chips.
>
> Acked-by: Nick Dyer <nick.dyer@itdev.co.uk>
>
> --
> Nick Dyer
> Software Engineer, ITDev Ltd
>
> Hardware and Software Development Consultancy
>
> Email: nick.dyer@itdev.co.uk
> Website: http://www.itdev.co.uk
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Input: atmel_mxt_ts - Use snprintf for sysfs attribute show method
2011-10-19 7:40 ` Daniel Kurtz
@ 2011-10-19 17:23 ` Dmitry Torokhov
0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Torokhov @ 2011-10-19 17:23 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Nick Dyer, Iiro Valkonen, Henrik Rydberg, Yufeng Shen,
linux-input, linux-kernel
Hi Daniel,
On Wednesday, October 19, 2011 12:40:16 AM Daniel Kurtz wrote:
> Hi Dmitry,
>
> I fear the following patch, and its 'Acked-by' from Nick Dyer, may
> have gotten lost:
No, it is not lost. It is commit 626af8611211c55595cd316103abd2419cd4d861
in my 'next' branch, waiting for 3.2 merge window to open.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 7+ messages in thread