Skip to contentSkip to conversation

staging: greybus: arche: use sysfs_emit() instead of sprintf()

PATCH Superseded Ruslan Valiyev 2 participants 4 messages Feb 25 – Feb 26

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: greybus: arche: use sysfs_emit() instead of sprintf()

Replace sprintf() with sysfs_emit() in state_show() sysfs attribute callbacks in arche-platform.c and arche-apb-ctrl.c.

sysfs_emit() is preferred over sprintf() in sysfs show functions because it is aware of the PAGE_SIZE buffer limit, preventing potential buffer overflows. This addresses checkpatch warnings about the use of sprintf() in sysfs show callbacks.

---
 drivers/staging/greybus/arche-apb-ctrl.c | 10 +++++-----
 drivers/staging/greybus/arche-platform.c | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-apb-ctrl.c +5 -5
index 33f26a65f..19a6e59b6 100644
@@ -300,16 +300,16 @@ static ssize_t state_show(struct device *dev,
 
 	switch (apb->state) {
 	case ARCHE_PLATFORM_STATE_OFF:
-		return sprintf(buf, "off%s\n",
+		return sysfs_emit(buf, "off%s\n",
 				apb->init_disabled ? ",disabled" : "");
 	case ARCHE_PLATFORM_STATE_ACTIVE:
-		return sprintf(buf, "active\n");
+		return sysfs_emit(buf, "active\n");
 	case ARCHE_PLATFORM_STATE_STANDBY:
-		return sprintf(buf, "standby\n");
+		return sysfs_emit(buf, "standby\n");
 	case ARCHE_PLATFORM_STATE_FW_FLASHING:
-		return sprintf(buf, "fw_flashing\n");
+		return sysfs_emit(buf, "fw_flashing\n");
 	default:
-		return sprintf(buf, "unknown state\n");
+		return sysfs_emit(buf, "unknown state\n");
 	}
 }
 
drivers/staging/greybus/arche-platform.c +5 -5
index f669a7e2e..de5de59ea 100644
@@ -374,15 +374,15 @@ static ssize_t state_show(struct device *dev,
 
 	switch (arche_pdata->state) {
 	case ARCHE_PLATFORM_STATE_OFF:
-		return sprintf(buf, "off\n");
+		return sysfs_emit(buf, "off\n");
 	case ARCHE_PLATFORM_STATE_ACTIVE:
-		return sprintf(buf, "active\n");
+		return sysfs_emit(buf, "active\n");
 	case ARCHE_PLATFORM_STATE_STANDBY:
-		return sprintf(buf, "standby\n");
+		return sysfs_emit(buf, "standby\n");
 	case ARCHE_PLATFORM_STATE_FW_FLASHING:
-		return sprintf(buf, "fw_flashing\n");
+		return sysfs_emit(buf, "fw_flashing\n");
 	default:
-		return sprintf(buf, "unknown state\n");
+		return sysfs_emit(buf, "unknown state\n");
 	}
 }
 
Signature · 1 line
2.43.0
#1 lore raw
Quoted 10 lines · Ruslan Valiyev

Replace sprintf() with sysfs_emit() in state_show() sysfs attribute callbacks in arche-platform.c and arche-apb-ctrl.c.

sysfs_emit() is preferred over sprintf() in sysfs show functions because it is aware of the PAGE_SIZE buffer limit, preventing potential buffer overflows. This addresses checkpatch warnings about the use of sprintf() in sysfs show callbacks.

---

I really want these sorts of patches to have some kind of analysis whether the "potential buffer overflows" are actual buffer overflows or not. In this case, it's really easy to see that there are no buffer overflows so it feels petty for something I can see just by glancing at the patch... I just got finished asking for the same thing from someone else, but in that case I had to look up the buffer size.

Anyway, just add a sentence at the start which says that "This code is obviously safe". And remove the bits which mention "potential buffer overflows".

Checkpatch complains about code using sprintf(). This code here is obviously safe as-is, but it would be more appropriate to use sysfs_emit().

regards,
dan carpenter

#2 lore raw

PATCH v2 1/1 staging: greybus: arche: use sysfs_emit() instead of sprintf()

In reply to Ruslan Valiyev

Replace sprintf() with sysfs_emit() in state_show() sysfs attribute callbacks in arche-platform.c and arche-apb-ctrl.c.

Checkpatch complains about code using sprintf(). This code here is obviously safe as-is, but it would be more appropriate to use sysfs_emit().

---
 drivers/staging/greybus/arche-apb-ctrl.c | 10 +++++-----
 drivers/staging/greybus/arche-platform.c | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/staging/greybus/arche-apb-ctrl.c +5 -5
index 33f26a65f..19a6e59b6 100644
@@ -300,16 +300,16 @@ static ssize_t state_show(struct device *dev,
 
 	switch (apb->state) {
 	case ARCHE_PLATFORM_STATE_OFF:
-		return sprintf(buf, "off%s\n",
+		return sysfs_emit(buf, "off%s\n",
 				apb->init_disabled ? ",disabled" : "");
 	case ARCHE_PLATFORM_STATE_ACTIVE:
-		return sprintf(buf, "active\n");
+		return sysfs_emit(buf, "active\n");
 	case ARCHE_PLATFORM_STATE_STANDBY:
-		return sprintf(buf, "standby\n");
+		return sysfs_emit(buf, "standby\n");
 	case ARCHE_PLATFORM_STATE_FW_FLASHING:
-		return sprintf(buf, "fw_flashing\n");
+		return sysfs_emit(buf, "fw_flashing\n");
 	default:
-		return sprintf(buf, "unknown state\n");
+		return sysfs_emit(buf, "unknown state\n");
 	}
 }
 
drivers/staging/greybus/arche-platform.c +5 -5
index f669a7e2e..de5de59ea 100644
@@ -374,15 +374,15 @@ static ssize_t state_show(struct device *dev,
 
 	switch (arche_pdata->state) {
 	case ARCHE_PLATFORM_STATE_OFF:
-		return sprintf(buf, "off\n");
+		return sysfs_emit(buf, "off\n");
 	case ARCHE_PLATFORM_STATE_ACTIVE:
-		return sprintf(buf, "active\n");
+		return sysfs_emit(buf, "active\n");
 	case ARCHE_PLATFORM_STATE_STANDBY:
-		return sprintf(buf, "standby\n");
+		return sysfs_emit(buf, "standby\n");
 	case ARCHE_PLATFORM_STATE_FW_FLASHING:
-		return sprintf(buf, "fw_flashing\n");
+		return sysfs_emit(buf, "fw_flashing\n");
 	default:
-		return sprintf(buf, "unknown state\n");
+		return sysfs_emit(buf, "unknown state\n");
 	}
 }
 
Signature · 1 line
2.43.0
Reviewed-by on v2 1/1
#3 lore raw
Quoted 9 lines · Ruslan Valiyev

Replace sprintf() with sysfs_emit() in state_show() sysfs attribute callbacks in arche-platform.c and arche-apb-ctrl.c.

Checkpatch complains about code using sprintf(). This code here is obviously safe as-is, but it would be more appropriate to use sysfs_emit().

---

Thanks!

regards,
dan carpenter