kasprintf() returns a pointer to dynamically allocated memory
which can be NULL upon failure.
Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
Cc: Kunwu Chan <[email protected]>
Signed-off-by: Kunwu Chan <[email protected]>
---
drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 64c4443dbef9..1b50d351f28b 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -1444,6 +1444,8 @@ static void iavf_print_link_message(struct iavf_adapter *adapter)
speed = kasprintf(GFP_KERNEL, "%d Mbps", link_speed_mbps);
}
+ if (!speed)
+ return;
netdev_info(netdev, "NIC Link is Up Speed is %s Full Duplex\n", speed);
kfree(speed);
}
--
2.39.2
On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote:
> kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure.
>
> Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
No need for the allocation here, print to a buffer on the stack.
--
pw-bot: cr
On 12/12/2023 1:28 PM, Jakub Kicinski wrote:
> On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote:
>> kasprintf() returns a pointer to dynamically allocated memory
>> which can be NULL upon failure.
>>
>> Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
>
> No need for the allocation here, print to a buffer on the stack.
Sure, but I think that just takes us full circle back to where we
started. reverting this to the previous code will add back W=1 warnings.
The whole point of the commit mentioned above was to get a reasonable
implementation that won't cause string truncation warnings. Is there
some trick I don't know about to get an allocation which will not
trigger snprintf and friends to print warnings from -Wformat-truncation
> drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1425:60: warning: ‘%s’ directive output may be truncated writing 4 bytes into a region of size between 1 and 11 [-Wformat-truncation=]
> drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1425:17: note: ‘snprintf’ output between 7 and 17 bytes into a destination of size 13
However the original warnings were for "%s" in strings. There should be
a good way, but I don't know it so could use some help.
-Jesse
On Tue, 12 Dec 2023 15:05:19 -0800 Jesse Brandeburg wrote:
> On 12/12/2023 1:28 PM, Jakub Kicinski wrote:
> > On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote:
> >> kasprintf() returns a pointer to dynamically allocated memory
> >> which can be NULL upon failure.
> >>
> >> Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
> >
> > No need for the allocation here, print to a buffer on the stack.
>
> Sure, but I think that just takes us full circle back to where we
> started. reverting this to the previous code will add back W=1 warnings.
>
> The whole point of the commit mentioned above was to get a reasonable
> implementation that won't cause string truncation warnings. Is there
> some trick I don't know about to get an allocation which will not
> trigger snprintf and friends to print warnings from -Wformat-truncation
Hm, it'd be nice if there was a flavor of snprintf which explicitly
doesn't trigger this warning. Or perhaps a marking for the output
buffer that says "truncation OK".
Absent that, can we print to a buffer on the stack and copy?
The link message is probably meh, but automation may get quite
confused if a NIC suddenly stops reporting FW version..
Thanks for your reply.
Sure, the only thing 'iavf_print_link_message' do is to print a msg by
netdev_info.
The 'iavf_virtchnl_completion' assume that no errors will be returned.
Whether we could just execute 'netdev_info(netdev, "NIC Link is Up Speed
is %s Full Duplex\n", speed? speed :"");' when 'speed' is null.
Before commit '1978d3ead82c8', the buffer size is '#define
IAVF_MAX_SPEED_STRLEN 13', whether we could use a bigger buffer
size to avoid a null pointer.
Such as '#define IAVF_MAX_SPEED_STRLEN 48'.
On 2023/12/13 05:28, Jakub Kicinski wrote:
> On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote:
>> kasprintf() returns a pointer to dynamically allocated memory
>> which can be NULL upon failure.
>>
>> Fixes: 1978d3ead82c ("intel: fix string truncation warnings")
>
> No need for the allocation here, print to a buffer on the stack.