diff mbox series

HID: hid-debug: clean up snprintf() checks in hid_resolv_usage()

Message ID 20210916132154.GC25094@kili (mailing list archive)
State Mainlined
Commit a68f3bd13994b315f47ec7e4da8d1c39ba0a2bb4
Delegated to: Jiri Kosina
Headers show
Series HID: hid-debug: clean up snprintf() checks in hid_resolv_usage() | expand

Commit Message

Dan Carpenter Sept. 16, 2021, 1:21 p.m. UTC
The snprintf() limits are complicated and slightly wrong when it does:

	max(0, HID_DEBUG_BUFSIZE - len - 1)

The "- 1" should not be there.  It means we can't use the last
byte of the buffer.  If we change the first snprintf() to scnprintf()
then we can remove the max().

At the start of the function the strlen(buf) is going always going to
be < HID_DEBUG_BUFSIZE so that is safe.  If it were > HID_DEBUG_BUFSIZE
then that would result in a WARN().

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/hid/hid-debug.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Jiri Kosina Sept. 22, 2021, 9:49 a.m. UTC | #1
On Thu, 16 Sep 2021, Dan Carpenter wrote:

> The snprintf() limits are complicated and slightly wrong when it does:
> 
> 	max(0, HID_DEBUG_BUFSIZE - len - 1)
> 
> The "- 1" should not be there.  It means we can't use the last
> byte of the buffer.  If we change the first snprintf() to scnprintf()
> then we can remove the max().
> 
> At the start of the function the strlen(buf) is going always going to
> be < HID_DEBUG_BUFSIZE so that is safe.  If it were > HID_DEBUG_BUFSIZE
> then that would result in a WARN().

Applied, thanks Dan.
diff mbox series

Patch

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index fa57d05badf7..3f62fe3b0a49 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -486,8 +486,7 @@  char *hid_resolv_usage(unsigned usage, struct seq_file *f) {
 
 	if (!f) {
 		len = strlen(buf);
-		snprintf(buf+len, max(0, HID_DEBUG_BUFSIZE - len), ".");
-		len++;
+		len += scnprintf(buf + len, HID_DEBUG_BUFSIZE - len, ".");
 	}
 	else {
 		seq_printf(f, ".");
@@ -498,7 +497,7 @@  char *hid_resolv_usage(unsigned usage, struct seq_file *f) {
 				if (p->usage == (usage & 0xffff)) {
 					if (!f)
 						snprintf(buf + len,
-							max(0,HID_DEBUG_BUFSIZE - len - 1),
+							HID_DEBUG_BUFSIZE - len,
 							"%s", p->description);
 					else
 						seq_printf(f,
@@ -509,8 +508,8 @@  char *hid_resolv_usage(unsigned usage, struct seq_file *f) {
 			break;
 		}
 	if (!f)
-		snprintf(buf + len, max(0, HID_DEBUG_BUFSIZE - len - 1),
-				"%04x", usage & 0xffff);
+		snprintf(buf + len, HID_DEBUG_BUFSIZE - len, "%04x",
+			 usage & 0xffff);
 	else
 		seq_printf(f, "%04x", usage & 0xffff);
 	return buf;