RE: [PATCH] net:realtek:use sysfs_emit() instead of scnprintf() for sysfs consistency

[Zong-Zhe Yang <kevin_yang@realtek.com>]

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(). !?