The Sun50i driver generates a warning with W=1:
warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=]
Fix it by allocating a big enough array to print an integer.
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/sun50i-cpufreq-nvmem.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
index 30e5c337611c..cd50cea16a87 100644
--- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
+++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
@@ -19,8 +19,6 @@
#include <linux/pm_opp.h>
#include <linux/slab.h>
-#define MAX_NAME_LEN 7
-
#define NVMEM_MASK 0x7
#define NVMEM_SHIFT 5
@@ -208,7 +206,7 @@ static int sun50i_cpufreq_get_efuse(void)
static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
{
int *opp_tokens;
- char name[MAX_NAME_LEN];
+ char name[] = "speedXXXXXXXXXXX"; /* Integers can take 11 chars max */
unsigned int cpu, supported_hw;
struct dev_pm_opp_config config = {};
int speed;
@@ -235,7 +233,7 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
config.supported_hw_count = 1;
}
- snprintf(name, MAX_NAME_LEN, "speed%d", speed);
+ snprintf(name, sizeof(name), "speed%d", speed);
config.prop_name = name;
for_each_possible_cpu(cpu) {
--
2.31.1.272.g89b43f80a514
On Sun, Apr 21, 2024 at 8:31 PM Viresh Kumar <[email protected]> wrote:
>
> The Sun50i driver generates a warning with W=1:
>
> warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=]
>
> Fix it by allocating a big enough array to print an integer.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Viresh Kumar <[email protected]>
Acked-by: Chen-Yu Tsai <[email protected]>
On Mon, 22 Apr 2024 09:01:09 +0530
Viresh Kumar <[email protected]> wrote:
Hi,
> The Sun50i driver generates a warning with W=1:
>
> warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=]
>
> Fix it by allocating a big enough array to print an integer.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Viresh Kumar <[email protected]>
For the records: as it stands right now, "speed" will always be smaller
than 10 at the moment, but it's indeed better to play safe here.
So the fix makes sense to me and fixes the issue:
Reviewed-by: Andre Przywara <[email protected]>
Tested-by: Andre Przywara <[email protected]>
Cheers,
Andre
> ---
> drivers/cpufreq/sun50i-cpufreq-nvmem.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> index 30e5c337611c..cd50cea16a87 100644
> --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> @@ -19,8 +19,6 @@
> #include <linux/pm_opp.h>
> #include <linux/slab.h>
>
> -#define MAX_NAME_LEN 7
> -
> #define NVMEM_MASK 0x7
> #define NVMEM_SHIFT 5
>
> @@ -208,7 +206,7 @@ static int sun50i_cpufreq_get_efuse(void)
> static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
> {
> int *opp_tokens;
> - char name[MAX_NAME_LEN];
> + char name[] = "speedXXXXXXXXXXX"; /* Integers can take 11 chars max */
> unsigned int cpu, supported_hw;
> struct dev_pm_opp_config config = {};
> int speed;
> @@ -235,7 +233,7 @@ static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
> config.supported_hw_count = 1;
> }
>
> - snprintf(name, MAX_NAME_LEN, "speed%d", speed);
> + snprintf(name, sizeof(name), "speed%d", speed);
> config.prop_name = name;
>
> for_each_possible_cpu(cpu) {
Hi Viresh,
On Mon, Apr 22, 2024 at 1:31 PM Viresh Kumar <[email protected]> wrote:
>
> The Sun50i driver generates a warning with W=1:
>
> warning: '%d' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=]
>
> Fix it by allocating a big enough array to print an integer.
>
> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Signed-off-by: Viresh Kumar <[email protected]>
This'll absolutely fix the issue here, so:
Reviewed-by: Julian Calaby <[email protected]>
However:
> ---
> drivers/cpufreq/sun50i-cpufreq-nvmem.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> index 30e5c337611c..cd50cea16a87 100644
> --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> @@ -208,7 +206,7 @@ static int sun50i_cpufreq_get_efuse(void)
> static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
> {
> int *opp_tokens;
> - char name[MAX_NAME_LEN];
> + char name[] = "speedXXXXXXXXXXX"; /* Integers can take 11 chars max */
Would it make sense to just set a static length for the string here,
say 17-20 characters and add a comment explaining the number, say: /*
"speed" + 11 chars for the int */
The string constant, while it'll probably be optimised away, seems
weird and wasteful.
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
On 23-04-24, 11:38, Julian Calaby wrote:
> On Mon, Apr 22, 2024 at 1:31 PM Viresh Kumar <[email protected]> wrote:
> > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > index 30e5c337611c..cd50cea16a87 100644
> > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > @@ -208,7 +206,7 @@ static int sun50i_cpufreq_get_efuse(void)
> > static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
> > {
> > int *opp_tokens;
> > - char name[MAX_NAME_LEN];
> > + char name[] = "speedXXXXXXXXXXX"; /* Integers can take 11 chars max */
>
> Would it make sense to just set a static length for the string here,
> say 17-20 characters and add a comment explaining the number, say: /*
> "speed" + 11 chars for the int */
>
> The string constant, while it'll probably be optimised away, seems
> weird and wasteful.
The counting goes wrong (I have done it in the past) sometimes and so
I like to explicitly reserve space like this, it also makes it look
cleaner, i.e. how the eventual string will be named.
--
viresh
Hi Viresh,
On Tue, Apr 23, 2024 at 4:12 PM Viresh Kumar <[email protected]> wrote:
>
> On 23-04-24, 11:38, Julian Calaby wrote:
> > On Mon, Apr 22, 2024 at 1:31 PM Viresh Kumar <[email protected]> wrote:
> > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > index 30e5c337611c..cd50cea16a87 100644
> > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > @@ -208,7 +206,7 @@ static int sun50i_cpufreq_get_efuse(void)
> > > static int sun50i_cpufreq_nvmem_probe(struct platform_device *pdev)
> > > {
> > > int *opp_tokens;
> > > - char name[MAX_NAME_LEN];
> > > + char name[] = "speedXXXXXXXXXXX"; /* Integers can take 11 chars max */
> >
> > Would it make sense to just set a static length for the string here,
> > say 17-20 characters and add a comment explaining the number, say: /*
> > "speed" + 11 chars for the int */
> >
> > The string constant, while it'll probably be optimised away, seems
> > weird and wasteful.
>
> The counting goes wrong (I have done it in the past) sometimes and so
> I like to explicitly reserve space like this, it also makes it look
> cleaner, i.e. how the eventual string will be named.
I completely agree - ultimately it's whatever works and either way
works equally well.
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/