linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency
@ 2025-07-24  5:50 Mande Imran Ahmed
  2025-07-24  8:42 ` Zong-Zhe Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Mande Imran Ahmed @ 2025-07-24  5:50 UTC (permalink / raw)
  To: pkshih, kevin_yang, rtl8821cerfe2, phhuang, damon.chen
  Cc: linux-kernel, Mande Imran Ahmed

Update the Realtek rtw89 wireless driver to replace scnprintf() with
sysfs_emit() for formatting sysfs attribute output, in line with the
recommendations from Documentation/filesystems/sysfs.rst.

This change enhances the safety and correctness of sysfs handling,
promotes consistency throughout the kernel, and aids long-term
maintainability.

Functionality verified using ping, iperf, and connection tests to ensure
stability after the change.

Signed-off-by: Mande Imran Ahmed <immu.ahmed1905@gmail.com>
---
 drivers/net/wireless/realtek/rtw89/phy.c |  8 +++----
 drivers/net/wireless/realtek/rtw89/sar.c | 30 ++++++++++++------------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index 76a2e26d4a10..a58aefb51fb5 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -2087,19 +2087,19 @@ EXPORT_SYMBOL(rtw89_phy_ant_gain_pwr_offset);
 int rtw89_print_ant_gain(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
 			 const struct rtw89_chan *chan)
 {
-	char *p = buf, *end = buf + bufsz;
+	char *p = buf;
 	s8 offset_patha, offset_pathb;
 
 	if (!rtw89_can_apply_ant_gain(rtwdev, chan->band_type)) {
-		p += scnprintf(p, end - p, "no DAG is applied\n");
+		p += sysfs_emit(p, "no DAG is applied\n");
 		goto out;
 	}
 
 	offset_patha = rtw89_phy_ant_gain_query(rtwdev, RF_PATH_A, chan->freq);
 	offset_pathb = rtw89_phy_ant_gain_query(rtwdev, RF_PATH_B, chan->freq);
 
-	p += scnprintf(p, end - p, "ChainA offset: %d dBm\n", offset_patha);
-	p += scnprintf(p, end - p, "ChainB offset: %d dBm\n", offset_pathb);
+	p += sysfs_emit(p, "ChainA offset: %d dBm\n", offset_patha);
+	p += sysfs_emit(p, "ChainB offset: %d dBm\n", offset_pathb);
 
 out:
 	return p - buf;
diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
index 517b66022f18..80eacada6911 100644
--- a/drivers/net/wireless/realtek/rtw89/sar.c
+++ b/drivers/net/wireless/realtek/rtw89/sar.c
@@ -318,7 +318,7 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
 	/* its members are protected by rtw89_sar_set_src() */
 	const struct rtw89_sar_handler *sar_hdl = &rtw89_sar_handlers[src];
 	const u8 fct_mac = rtwdev->chip->txpwr_factor_mac;
-	char *p = buf, *end = buf + bufsz;
+	char *p = buf;
 	int ret;
 	s32 cfg;
 	u8 fct;
@@ -326,17 +326,17 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
 	lockdep_assert_wiphy(rtwdev->hw->wiphy);
 
 	if (src == RTW89_SAR_SOURCE_NONE) {
-		p += scnprintf(p, end - p, "no SAR is applied\n");
+		p += sysfs_emit(p, "no SAR is applied\n");
 		goto out;
 	}
 
-	p += scnprintf(p, end - p, "source: %d (%s)\n", src,
+	p += sysfs_emit(p, "source: %d (%s)\n", src,
 		       sar_hdl->descr_sar_source);
 
 	ret = sar_hdl->query_sar_config(rtwdev, sar_parm, &cfg);
 	if (ret) {
-		p += scnprintf(p, end - p, "config: return code: %d\n", ret);
-		p += scnprintf(p, end - p,
+		p += sysfs_emit(p, "config: return code: %d\n", ret);
+		p += sysfs_emit(p,
 			       "assign: max setting: %d (unit: 1/%lu dBm)\n",
 			       RTW89_SAR_TXPWR_MAC_MAX, BIT(fct_mac));
 		goto out;
@@ -344,10 +344,10 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
 
 	fct = sar_hdl->txpwr_factor_sar;
 
-	p += scnprintf(p, end - p, "config: %d (unit: 1/%lu dBm)\n", cfg,
+	p += sysfs_emit(p, "config: %d (unit: 1/%lu dBm)\n", cfg,
 		       BIT(fct));
 
-	p += scnprintf(p, end - p, "support different configs by antenna: %s\n",
+	p += sysfs_emit(p, "support different configs by antenna: %s\n",
 		       str_yes_no(rtwdev->chip->support_sar_by_ant));
 out:
 	return p - buf;
@@ -356,24 +356,24 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
 int rtw89_print_tas(struct rtw89_dev *rtwdev, char *buf, size_t bufsz)
 {
 	struct rtw89_tas_info *tas = &rtwdev->tas;
-	char *p = buf, *end = buf + bufsz;
+	char *p = buf;
 
 	if (!rtw89_tas_is_active(rtwdev)) {
-		p += scnprintf(p, end - p, "no TAS is applied\n");
+		p += sysfs_emit(p, "no TAS is applied\n");
 		goto out;
 	}
 
-	p += scnprintf(p, end - p, "State: %s\n",
+	p += sysfs_emit(p, "State: %s\n",
 		       rtw89_tas_state_str(tas->state));
-	p += scnprintf(p, end - p, "Average time: %d\n",
+	p += sysfs_emit(p, "Average time: %d\n",
 		       tas->window_size * 2);
-	p += scnprintf(p, end - p, "SAR gap: %d dBm\n",
+	p += sysfs_emit(p, "SAR gap: %d dBm\n",
 		       RTW89_TAS_SAR_GAP >> RTW89_TAS_FACTOR);
-	p += scnprintf(p, end - p, "DPR gap: %d dBm\n",
+	p += sysfs_emit(p, "DPR gap: %d dBm\n",
 		       RTW89_TAS_DPR_GAP >> RTW89_TAS_FACTOR);
-	p += scnprintf(p, end - p, "DPR ON offset: %d dBm\n",
+	p += sysfs_emit(p, "DPR ON offset: %d dBm\n",
 		       RTW89_TAS_DPR_ON_OFFSET >> RTW89_TAS_FACTOR);
-	p += scnprintf(p, end - p, "DPR OFF offset: %d dBm\n",
+	p += sysfs_emit(p, "DPR OFF offset: %d dBm\n",
 		       RTW89_TAS_DPR_OFF_OFFSET >> RTW89_TAS_FACTOR);
 
 out:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency
  2025-07-24  5:50 [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency Mande Imran Ahmed
@ 2025-07-24  8:42 ` Zong-Zhe Yang
  2025-07-24  8:57   ` Ping-Ke Shih
  2025-07-28 11:26   ` [PATCH v2] " Mande Imran Ahmed
  0 siblings, 2 replies; 6+ messages in thread
From: Zong-Zhe Yang @ 2025-07-24  8:42 UTC (permalink / raw)
  To: Mande Imran Ahmed, Ping-Ke Shih, rtl8821cerfe2@gmail.com,
	Bernie Huang, Damon Chen
  Cc: linux-kernel@vger.kernel.org

Mande Imran Ahmed <immu.ahmed1905@gmail.com> wrote:
> 
> Update the Realtek rtw89 wireless driver to replace scnprintf() with
> sysfs_emit() for formatting sysfs attribute output, in line with the recommendations from
> Documentation/filesystems/sysfs.rst.
> 
> This change enhances the safety and correctness of sysfs handling, promotes consistency
> throughout the kernel, and aids long-term maintainability.
> 
> Functionality verified using ping, iperf, and connection tests to ensure stability after the
> change.
> 
> Signed-off-by: Mande Imran Ahmed <immu.ahmed1905@gmail.com>
> ---
>  drivers/net/wireless/realtek/rtw89/phy.c |  8 +++----
> drivers/net/wireless/realtek/rtw89/sar.c | 30 ++++++++++++------------
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw89/phy.c
> b/drivers/net/wireless/realtek/rtw89/phy.c
> index 76a2e26d4a10..a58aefb51fb5 100644
> --- a/drivers/net/wireless/realtek/rtw89/phy.c
> +++ b/drivers/net/wireless/realtek/rtw89/phy.c
> @@ -2087,19 +2087,19 @@ EXPORT_SYMBOL(rtw89_phy_ant_gain_pwr_offset);
>  int rtw89_print_ant_gain(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
>                          const struct rtw89_chan *chan)  {
> -       char *p = buf, *end = buf + bufsz;
> +       char *p = buf;
>         s8 offset_patha, offset_pathb;
> 
>         if (!rtw89_can_apply_ant_gain(rtwdev, chan->band_type)) {
> -               p += scnprintf(p, end - p, "no DAG is applied\n");
> +               p += sysfs_emit(p, "no DAG is applied\n");
>                 goto out;
>         }
> 
>         offset_patha = rtw89_phy_ant_gain_query(rtwdev, RF_PATH_A, chan->freq);
>         offset_pathb = rtw89_phy_ant_gain_query(rtwdev, RF_PATH_B, chan->freq);
> 
> -       p += scnprintf(p, end - p, "ChainA offset: %d dBm\n", offset_patha);
> -       p += scnprintf(p, end - p, "ChainB offset: %d dBm\n", offset_pathb);
> +       p += sysfs_emit(p, "ChainA offset: %d dBm\n", offset_patha);
> +       p += sysfs_emit(p, "ChainB offset: %d dBm\n", offset_pathb);
> 
>  out:
>         return p - buf;
> diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
> index 517b66022f18..80eacada6911 100644
> --- a/drivers/net/wireless/realtek/rtw89/sar.c
> +++ b/drivers/net/wireless/realtek/rtw89/sar.c
> @@ -318,7 +318,7 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
>         /* its members are protected by rtw89_sar_set_src() */
>         const struct rtw89_sar_handler *sar_hdl = &rtw89_sar_handlers[src];
>         const u8 fct_mac = rtwdev->chip->txpwr_factor_mac;
> -       char *p = buf, *end = buf + bufsz;
> +       char *p = buf;
>         int ret;
>         s32 cfg;
>         u8 fct;
> @@ -326,17 +326,17 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t
> bufsz,
>         lockdep_assert_wiphy(rtwdev->hw->wiphy);
> 
>         if (src == RTW89_SAR_SOURCE_NONE) {
> -               p += scnprintf(p, end - p, "no SAR is applied\n");
> +               p += sysfs_emit(p, "no SAR is applied\n");
>                 goto out;
>         }
> 
> -       p += scnprintf(p, end - p, "source: %d (%s)\n", src,
> +       p += sysfs_emit(p, "source: %d (%s)\n", src,
>                        sar_hdl->descr_sar_source);
> 
>         ret = sar_hdl->query_sar_config(rtwdev, sar_parm, &cfg);
>         if (ret) {
> -               p += scnprintf(p, end - p, "config: return code: %d\n", ret);
> -               p += scnprintf(p, end - p,
> +               p += sysfs_emit(p, "config: return code: %d\n", ret);
> +               p += sysfs_emit(p,
>                                "assign: max setting: %d (unit: 1/%lu dBm)\n",
>                                RTW89_SAR_TXPWR_MAC_MAX, BIT(fct_mac));
>                 goto out;
> @@ -344,10 +344,10 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t
> bufsz,
> 
>         fct = sar_hdl->txpwr_factor_sar;
> 
> -       p += scnprintf(p, end - p, "config: %d (unit: 1/%lu dBm)\n", cfg,
> +       p += sysfs_emit(p, "config: %d (unit: 1/%lu dBm)\n", cfg,
>                        BIT(fct));
> 
> -       p += scnprintf(p, end - p, "support different configs by antenna: %s\n",
> +       p += sysfs_emit(p, "support different configs by antenna: %s\n",
>                        str_yes_no(rtwdev->chip->support_sar_by_ant));
>  out:
>         return p - buf;
> @@ -356,24 +356,24 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t
> bufsz,  int rtw89_print_tas(struct rtw89_dev *rtwdev, char *buf, size_t bufsz)  {
>         struct rtw89_tas_info *tas = &rtwdev->tas;
> -       char *p = buf, *end = buf + bufsz;
> +       char *p = buf;
> 
>         if (!rtw89_tas_is_active(rtwdev)) {
> -               p += scnprintf(p, end - p, "no TAS is applied\n");
> +               p += sysfs_emit(p, "no TAS is applied\n");
>                 goto out;
>         }
> 
> -       p += scnprintf(p, end - p, "State: %s\n",
> +       p += sysfs_emit(p, "State: %s\n",
>                        rtw89_tas_state_str(tas->state));
> -       p += scnprintf(p, end - p, "Average time: %d\n",
> +       p += sysfs_emit(p, "Average time: %d\n",
>                        tas->window_size * 2);
> -       p += scnprintf(p, end - p, "SAR gap: %d dBm\n",
> +       p += sysfs_emit(p, "SAR gap: %d dBm\n",
>                        RTW89_TAS_SAR_GAP >> RTW89_TAS_FACTOR);
> -       p += scnprintf(p, end - p, "DPR gap: %d dBm\n",
> +       p += sysfs_emit(p, "DPR gap: %d dBm\n",
>                        RTW89_TAS_DPR_GAP >> RTW89_TAS_FACTOR);
> -       p += scnprintf(p, end - p, "DPR ON offset: %d dBm\n",
> +       p += sysfs_emit(p, "DPR ON offset: %d dBm\n",
>                        RTW89_TAS_DPR_ON_OFFSET >> RTW89_TAS_FACTOR);
> -       p += scnprintf(p, end - p, "DPR OFF offset: %d dBm\n",
> +       p += sysfs_emit(p, "DPR OFF offset: %d dBm\n",
>                        RTW89_TAS_DPR_OFF_OFFSET >> RTW89_TAS_FACTOR);
> 
>  out:
> --
> 2.43.0

(1.) buffer might not just be allocated with PAGE_SIZE
(2.) the pointer passed to leaf function might not point to the head of allocated buffer

Will the above cause some problems ?

For (2.), maybe need to tweak them with sysfs_emit_at() instead of sysfs_emit(). !?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency
  2025-07-24  8:42 ` Zong-Zhe Yang
@ 2025-07-24  8:57   ` Ping-Ke Shih
  2025-07-28 11:31     ` Mande Imran Ahmed
  2025-07-28 11:26   ` [PATCH v2] " Mande Imran Ahmed
  1 sibling, 1 reply; 6+ messages in thread
From: Ping-Ke Shih @ 2025-07-24  8:57 UTC (permalink / raw)
  To: Zong-Zhe Yang, Mande Imran Ahmed, rtl8821cerfe2@gmail.com,
	Bernie Huang, Damon Chen
  Cc: linux-kernel@vger.kernel.org

Zong-Zhe Yang <kevin_yang@realtek.com> wrote:
> Mande Imran Ahmed <immu.ahmed1905@gmail.com> wrote:
> >
> > Update the Realtek rtw89 wireless driver to replace scnprintf() with
> > sysfs_emit() for formatting sysfs attribute output, in line with the recommendations from
> > Documentation/filesystems/sysfs.rst.

This is for sysfs, no? But this patch is related to debugfs, which buffer
size isn't PAGE_SIZE. Please refer to implementation of sysfs_emit().

So NACK.

> >
> > This change enhances the safety and correctness of sysfs handling, promotes consistency
> > throughout the kernel, and aids long-term maintainability.
> >
> > Functionality verified using ping, iperf, and connection tests to ensure stability after the
> > change.
> >
> > Signed-off-by: Mande Imran Ahmed <immu.ahmed1905@gmail.com>

[...]

> 
> (1.) buffer might not just be allocated with PAGE_SIZE
> (2.) the pointer passed to leaf function might not point to the head of allocated buffer
> 
> Will the above cause some problems ?
> 
> For (2.), maybe need to tweak them with sysfs_emit_at() instead of sysfs_emit(). !?

The assumption of buffer size in sysfs_emit() is PAGE_SIZE, but this is
totally wrong in rtw89 debugfs.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency
  2025-07-24  8:42 ` Zong-Zhe Yang
  2025-07-24  8:57   ` Ping-Ke Shih
@ 2025-07-28 11:26   ` Mande Imran Ahmed
  2025-07-29  1:33     ` Ping-Ke Shih
  1 sibling, 1 reply; 6+ messages in thread
From: Mande Imran Ahmed @ 2025-07-28 11:26 UTC (permalink / raw)
  To: pkshih, kevin_yang, rtl8821cerfe2, phhuang, damon.chen
  Cc: linux-kernel, Mande Imran Ahmed

Update the Realtek rtw89 wireless driver to replace scnprintf() with
sysfs_emit() for formatting sysfs attribute output, in line with the
recommendations from Documentation/filesystems/sysfs.rst.

This change enhances the safety and correctness of sysfs handling,
promotes consistency throughout the kernel, and aids long-term
maintainability.

Functionality verified using ping, iperf, and connection tests to ensure
stability after the changes.

Changes since v1:
- Replaced sysfs_emit() with sysfs_emit_at()
- Removed unused function parameters.

Signed-off-by: Mande Imran Ahmed <immu.ahmed1905@gmail.com>
---
 drivers/net/wireless/realtek/rtw89/debug.c |  4 +-
 drivers/net/wireless/realtek/rtw89/phy.c   | 12 ++---
 drivers/net/wireless/realtek/rtw89/phy.h   |  2 +-
 drivers/net/wireless/realtek/rtw89/sar.c   | 58 +++++++++++-----------
 drivers/net/wireless/realtek/rtw89/sar.h   |  2 +-
 5 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/debug.c b/drivers/net/wireless/realtek/rtw89/debug.c
index d6016fa107fb..74a8e0976715 100644
--- a/drivers/net/wireless/realtek/rtw89/debug.c
+++ b/drivers/net/wireless/realtek/rtw89/debug.c
@@ -941,13 +941,13 @@ ssize_t rtw89_debug_priv_txpwr_table_get(struct rtw89_dev *rtwdev,
 	p += rtw89_debug_priv_txpwr_table_get_regd(rtwdev, p, end - p, chan);
 
 	p += scnprintf(p, end - p, "[SAR]\n");
-	p += rtw89_print_sar(rtwdev, p, end - p, &sar_parm);
+	p += rtw89_print_sar(rtwdev, p, &sar_parm);
 
 	p += scnprintf(p, end - p, "[TAS]\n");
 	p += rtw89_print_tas(rtwdev, p, end - p);
 
 	p += scnprintf(p, end - p, "[DAG]\n");
-	p += rtw89_print_ant_gain(rtwdev, p, end - p, chan);
+	p += rtw89_print_ant_gain(rtwdev, p, chan);
 
 	tbl = dbgfs_txpwr_tables[chip_gen];
 	if (!tbl)
diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index a58aefb51fb5..1fa2c7a04bd7 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -2084,25 +2084,25 @@ s16 rtw89_phy_ant_gain_pwr_offset(struct rtw89_dev *rtwdev,
 }
 EXPORT_SYMBOL(rtw89_phy_ant_gain_pwr_offset);
 
-int rtw89_print_ant_gain(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
+int rtw89_print_ant_gain(struct rtw89_dev *rtwdev, char *buf,
 			 const struct rtw89_chan *chan)
 {
-	char *p = buf;
+	int len;
 	s8 offset_patha, offset_pathb;
 
 	if (!rtw89_can_apply_ant_gain(rtwdev, chan->band_type)) {
-		p += sysfs_emit(p, "no DAG is applied\n");
+		len = sysfs_emit(buf, "no DAG is applied\n");
 		goto out;
 	}
 
 	offset_patha = rtw89_phy_ant_gain_query(rtwdev, RF_PATH_A, chan->freq);
 	offset_pathb = rtw89_phy_ant_gain_query(rtwdev, RF_PATH_B, chan->freq);
 
-	p += sysfs_emit(p, "ChainA offset: %d dBm\n", offset_patha);
-	p += sysfs_emit(p, "ChainB offset: %d dBm\n", offset_pathb);
+	len = sysfs_emit(buf, "ChainA offset: %d dBm\n", offset_patha);
+	len += sysfs_emit_at(buf, len, "ChainB offset: %d dBm\n", offset_pathb);
 
 out:
-	return p - buf;
+	return len;
 }
 
 static const u8 rtw89_rs_idx_num_ax[] = {
diff --git a/drivers/net/wireless/realtek/rtw89/phy.h b/drivers/net/wireless/realtek/rtw89/phy.h
index 5b451f1cfaac..f5518f5bc668 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.h
+++ b/drivers/net/wireless/realtek/rtw89/phy.h
@@ -836,7 +836,7 @@ s8 rtw89_phy_read_txpwr_byrate(struct rtw89_dev *rtwdev, u8 band, u8 bw,
 void rtw89_phy_ant_gain_init(struct rtw89_dev *rtwdev);
 s16 rtw89_phy_ant_gain_pwr_offset(struct rtw89_dev *rtwdev,
 				  const struct rtw89_chan *chan);
-int rtw89_print_ant_gain(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
+int rtw89_print_ant_gain(struct rtw89_dev *rtwdev, char *buf,
 			 const struct rtw89_chan *chan);
 void rtw89_phy_load_txpwr_byrate(struct rtw89_dev *rtwdev,
 				 const struct rtw89_txpwr_table *tbl);
diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
index 80eacada6911..14cefbe64124 100644
--- a/drivers/net/wireless/realtek/rtw89/sar.c
+++ b/drivers/net/wireless/realtek/rtw89/sar.c
@@ -311,14 +311,14 @@ s8 rtw89_query_sar(struct rtw89_dev *rtwdev, const struct rtw89_sar_parm *sar_pa
 }
 EXPORT_SYMBOL(rtw89_query_sar);
 
-int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
+int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf,
 		    const struct rtw89_sar_parm *sar_parm)
 {
 	const enum rtw89_sar_sources src = rtwdev->sar.src;
 	/* its members are protected by rtw89_sar_set_src() */
 	const struct rtw89_sar_handler *sar_hdl = &rtw89_sar_handlers[src];
 	const u8 fct_mac = rtwdev->chip->txpwr_factor_mac;
-	char *p = buf;
+	int len;
 	int ret;
 	s32 cfg;
 	u8 fct;
@@ -326,58 +326,58 @@ int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
 	lockdep_assert_wiphy(rtwdev->hw->wiphy);
 
 	if (src == RTW89_SAR_SOURCE_NONE) {
-		p += sysfs_emit(p, "no SAR is applied\n");
+		len = sysfs_emit(buf, "no SAR is applied\n");
 		goto out;
 	}
 
-	p += sysfs_emit(p, "source: %d (%s)\n", src,
-		       sar_hdl->descr_sar_source);
+	len = sysfs_emit(buf, "source: %d (%s)\n", src,
+			 sar_hdl->descr_sar_source);
 
 	ret = sar_hdl->query_sar_config(rtwdev, sar_parm, &cfg);
 	if (ret) {
-		p += sysfs_emit(p, "config: return code: %d\n", ret);
-		p += sysfs_emit(p,
-			       "assign: max setting: %d (unit: 1/%lu dBm)\n",
-			       RTW89_SAR_TXPWR_MAC_MAX, BIT(fct_mac));
+		len += sysfs_emit_at(buf, len, "config: return code: %d\n", ret);
+		len += sysfs_emit_at(buf, len,
+				"assign: max setting: %d (unit: 1/%lu dBm)\n",
+				RTW89_SAR_TXPWR_MAC_MAX, BIT(fct_mac));
 		goto out;
 	}
 
 	fct = sar_hdl->txpwr_factor_sar;
 
-	p += sysfs_emit(p, "config: %d (unit: 1/%lu dBm)\n", cfg,
-		       BIT(fct));
+	len += sysfs_emit_at(buf, len, "config: %d (unit: 1/%lu dBm)\n", cfg,
+			BIT(fct));
 
-	p += sysfs_emit(p, "support different configs by antenna: %s\n",
-		       str_yes_no(rtwdev->chip->support_sar_by_ant));
+	len += sysfs_emit_at(buf, len, "support different configs by antenna: %s\n",
+			str_yes_no(rtwdev->chip->support_sar_by_ant));
 out:
-	return p - buf;
+	return len;
 }
 
 int rtw89_print_tas(struct rtw89_dev *rtwdev, char *buf, size_t bufsz)
 {
 	struct rtw89_tas_info *tas = &rtwdev->tas;
-	char *p = buf;
+	int len;
 
 	if (!rtw89_tas_is_active(rtwdev)) {
-		p += sysfs_emit(p, "no TAS is applied\n");
+		len = sysfs_emit(buf, "no TAS is applied\n");
 		goto out;
 	}
 
-	p += sysfs_emit(p, "State: %s\n",
-		       rtw89_tas_state_str(tas->state));
-	p += sysfs_emit(p, "Average time: %d\n",
-		       tas->window_size * 2);
-	p += sysfs_emit(p, "SAR gap: %d dBm\n",
-		       RTW89_TAS_SAR_GAP >> RTW89_TAS_FACTOR);
-	p += sysfs_emit(p, "DPR gap: %d dBm\n",
-		       RTW89_TAS_DPR_GAP >> RTW89_TAS_FACTOR);
-	p += sysfs_emit(p, "DPR ON offset: %d dBm\n",
-		       RTW89_TAS_DPR_ON_OFFSET >> RTW89_TAS_FACTOR);
-	p += sysfs_emit(p, "DPR OFF offset: %d dBm\n",
-		       RTW89_TAS_DPR_OFF_OFFSET >> RTW89_TAS_FACTOR);
+	len = sysfs_emit(buf, "State: %s\n",
+			 rtw89_tas_state_str(tas->state));
+	len += sysfs_emit_at(buf, len, "Average time: %d\n",
+			tas->window_size * 2);
+	len += sysfs_emit_at(buf, len, "SAR gap: %d dBm\n",
+			RTW89_TAS_SAR_GAP >> RTW89_TAS_FACTOR);
+	len += sysfs_emit_at(buf, len, "DPR gap: %d dBm\n",
+			RTW89_TAS_DPR_GAP >> RTW89_TAS_FACTOR);
+	len += sysfs_emit_at(buf, len, "DPR ON offset: %d dBm\n",
+			RTW89_TAS_DPR_ON_OFFSET >> RTW89_TAS_FACTOR);
+	len += sysfs_emit_at(buf, len, "DPR OFF offset: %d dBm\n",
+			RTW89_TAS_DPR_OFF_OFFSET >> RTW89_TAS_FACTOR);
 
 out:
-	return p - buf;
+	return len;
 }
 
 static int rtw89_apply_sar_common(struct rtw89_dev *rtwdev,
diff --git a/drivers/net/wireless/realtek/rtw89/sar.h b/drivers/net/wireless/realtek/rtw89/sar.h
index 4b7f3d44f57b..b84277a3ea38 100644
--- a/drivers/net/wireless/realtek/rtw89/sar.h
+++ b/drivers/net/wireless/realtek/rtw89/sar.h
@@ -28,7 +28,7 @@ struct rtw89_sar_handler {
 extern const struct cfg80211_sar_capa rtw89_sar_capa;
 
 s8 rtw89_query_sar(struct rtw89_dev *rtwdev, const struct rtw89_sar_parm *sar_parm);
-int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf, size_t bufsz,
+int rtw89_print_sar(struct rtw89_dev *rtwdev, char *buf,
 		    const struct rtw89_sar_parm *sar_parm);
 int rtw89_print_tas(struct rtw89_dev *rtwdev, char *buf, size_t bufsz);
 int rtw89_ops_set_sar_specs(struct ieee80211_hw *hw,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency
  2025-07-24  8:57   ` Ping-Ke Shih
@ 2025-07-28 11:31     ` Mande Imran Ahmed
  0 siblings, 0 replies; 6+ messages in thread
From: Mande Imran Ahmed @ 2025-07-28 11:31 UTC (permalink / raw)
  To: pkshih, kevin_yang, rtl8821cerfe2, phhuang, damon.chen
  Cc: linux-kernel, Mande Imran Ahmed


The implementation of sysfs_emit() internally uses vscnprintf(buf, PAGE_SIZE, fmt, args), which assumes the buffer size is always PAGE_SIZE. This assumption is valid in sysfs, where the buffer passed to the read function is guaranteed to be PAGE_SIZE.

Using sysfs_emit_at() is a safer alternative when we need to control the offset, but both functions still rely on that fixed buffer size assumption. They are designed specifically for the sysfs environment, where such constraints are well-defined.

sysfs_emit_at() is optimized to work directly with sysfs interface for better performance and can avoid unnecessary overhead related to formatting the output.
With sysfs_emit_at() we can avoid overflow error's by using the fixed size of PAGE_SIZE.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v2] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency
  2025-07-28 11:26   ` [PATCH v2] " Mande Imran Ahmed
@ 2025-07-29  1:33     ` Ping-Ke Shih
  0 siblings, 0 replies; 6+ messages in thread
From: Ping-Ke Shih @ 2025-07-29  1:33 UTC (permalink / raw)
  To: Mande Imran Ahmed, Zong-Zhe Yang, rtl8821cerfe2@gmail.com,
	Bernie Huang, Damon Chen
  Cc: linux-kernel@vger.kernel.org

Mande Imran Ahmed <immu.ahmed1905@gmail.com> wrote:
> Update the Realtek rtw89 wireless driver to replace scnprintf() with
> sysfs_emit() for formatting sysfs attribute output, in line with the
> recommendations from Documentation/filesystems/sysfs.rst.

As mentioned in v1. This is debugfs, not sysfs. The assumption of PAGE_SIZE
buffer can't be guaranteed. More, this driver must not depend on sysfs.

NACK. Please do not try to respin patch by v3 I'll ignore it.

> 
> This change enhances the safety and correctness of sysfs handling,
> promotes consistency throughout the kernel, and aids long-term
> maintainability.
> 
> Functionality verified using ping, iperf, and connection tests to ensure
> stability after the changes.
> 
> Changes since v1:
> - Replaced sysfs_emit() with sysfs_emit_at()
> - Removed unused function parameters.
> 
> Signed-off-by: Mande Imran Ahmed <immu.ahmed1905@gmail.com>

[...]


> 
> -       p += sysfs_emit(p, "ChainA offset: %d dBm\n", offset_patha);
> -       p += sysfs_emit(p, "ChainB offset: %d dBm\n", offset_pathb);

By the way, the original driver never uses sysfs_emit(). 

> +       len = sysfs_emit(buf, "ChainA offset: %d dBm\n", offset_patha);
> +       len += sysfs_emit_at(buf, len, "ChainB offset: %d dBm\n", offset_pathb);

[...]


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-29  1:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-24  5:50 [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency Mande Imran Ahmed
2025-07-24  8:42 ` Zong-Zhe Yang
2025-07-24  8:57   ` Ping-Ke Shih
2025-07-28 11:31     ` Mande Imran Ahmed
2025-07-28 11:26   ` [PATCH v2] " Mande Imran Ahmed
2025-07-29  1:33     ` Ping-Ke Shih

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).