Re: [Patch v3 2/2] memory: tegra: make sid and broadcast regions optional

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





     static inline u32 mc_readl(const struct tegra_mc *mc, unsigned long offset)
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 1b3183951bfe..716582255eeb 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -26,20 +26,16 @@
     static int tegra186_mc_probe(struct tegra_mc *mc)
     {
          struct platform_device *pdev = to_platform_device(mc->dev);
+     struct resource *res;
          unsigned int i;
-     char name[8];
+     char name[14];

How is it relevant? I don't see this being used in your diff.


Best regards,
Krzysztof


Did this change for below warning coming with 'W=1'.

../drivers/memory/tegra/tegra186.c: In function tegra186_mc_probe:
../drivers/memory/tegra/tegra186.c:51:49: warning: %u directive output
may be truncated writing between 1 and 10 bytes into a region of size 6
[8;;https://gc
c.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-truncation=-Wformat-truncation=8;;]
       51 |                 snprintf(name, sizeof(name), "ch%u", i);
          |                                                 ^~
../drivers/memory/tegra/tegra186.c:51:46: note: directive argument in
the range [0, 4294967294]
       51 |                 snprintf(name, sizeof(name), "ch%u", i);
          |                                              ^~~~~~
../drivers/memory/tegra/tegra186.c:51:17: note: snprintf output between
4 and 13 bytes into a destination of size 8
       51 |                 snprintf(name, sizeof(name), "ch%u", i);
          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I asked how this is relevant to this change and you answer there is a
warning. If the warning was there, your answer is really just deflecting
the topic, so obviously this is new warning. Which part of code uses
longer name?

BTW, really, such answers do not make review of your code smoother.

Best regards,
Krzysztof


Apologies for not explaining it earlier.

I increased the buffer size to suppress a static check warning in the
existing code due to big range of 'unsigned int i', if copied to small
name buffer.

Seems like the warning is harmless as the maximum value of num_channels
is 16. I will remove it and keep the buffer size as 8 in the next
version.


That's not the point. For the third time: how is it relevant to this
change here? Was or was not the warning before?


This is not relevant to the change here. The warning was before as well.

OK, fixing the warning is always a good idea, but this *must* be always
separate patch, with its own explanation and rationale, and warning message.


Sure, will submit a separate patch for the warning and spin a v4 for this patch series after incorporating all review comments.

Thank you,
Sumit Gupta

Best regards,
Krzysztof





[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux