Re: [PATCH] [v2] hinic: avoid gcc -Wrestrict warning

On Wed, Mar 24, 2021 at 2:29 PM Rasmus Villemoes
[off-list ref] wrote:

On 24/03/2021 14.07, Arnd Bergmann wrote:

 >

quoted

      if (set_settings & HILINK_LINK_SET_SPEED) {
              speed_level = hinic_ethtool_to_hw_speed_level(speed);
              err = snprintf(set_link_str, SET_LINK_STR_MAX_LEN,
-                            "%sspeed %d ", set_link_str, speed);
-             if (err <= 0 || err >= SET_LINK_STR_MAX_LEN) {
+                            "speed %d ", speed);
+             if (err >= SET_LINK_STR_MAX_LEN) {
                      netif_err(nic_dev, drv, netdev, "Failed to snprintf link speed, function return(%d) and dest_len(%d)\n",
                                err, SET_LINK_STR_MAX_LEN);
                      return -EFAULT;

It's not your invention of course, but this both seems needlessly harsh
and EFAULT is a weird error to return. It's just a printk() message that
might be truncated, and now that the format string only has a %d
specifier, it can actually be verified statically that overflow will
never happen (though I don't know or think gcc can do that, perhaps
there's some locale nonsense in the standard that allows using
utf16-encoded sanskrit runes). So probably that test should just be
dropped, but that's a separate thing.

I thought about fixing it, but this seemed to be a rabbit hole I didn't
want to get into, as there are other harmless issues in the driver
that could be improved.

I'm fairly sure gcc can indeed warn about the overflow with
-Wformat-truncation, but the warning is disabled at the moment
because it has a ton of false positives:

kernel/workqueue.c: In function 'create_worker':
kernel/workqueue.c:1933:55: error: '%d' directive output may be
truncated writing between 1 and 10 bytes into a region of size between
3 and 13 [-Werror=format-truncation=]
 1933 |                 snprintf(id_buf, sizeof(id_buf), "u%d:%d",
pool->id, id);

        Arnd