On 2025/8/4 16:21, Dan Carpenter wrote:
Hello Lance Yang, Commit e89a68046687 ("netfilter: load nf_log_syslog on enabling nf_conntrack_log_invalid") from May 26, 2025 (linux-next), leads to the following Smatch static checker warning: net/netfilter/nf_conntrack_standalone.c:575 nf_conntrack_log_invalid_sysctl() warn: missing error code? 'ret'
Thanks for pointing this out!
net/netfilter/nf_conntrack_standalone.c 559 static int 560 nf_conntrack_log_invalid_sysctl(const struct ctl_table *table, int write, 561 void *buffer, size_t *lenp, loff_t *ppos) 562 { 563 int ret, i; 564 565 ret = proc_dou8vec_minmax(table, write, buffer, lenp, ppos); 566 if (ret < 0 || !write) 567 return ret; 568 569 if (*(u8 *)table->data == 0) 570 return ret; return 0?
That's a good question. proc_dou8vec_minmax() returns 0 on a successful write. So when a user writes '0' to disable the feature, ret is already 0. Returning it is the correct behavior to signal success.
571 572 /* Load nf_log_syslog only if no logger is currently registered */ 573 for (i = 0; i < NFPROTO_NUMPROTO; i++) { 574 if (nf_log_is_registered(i)) --> 575 return ret; This feels like it should be return -EBUSY? Or potentially return 0.
We simply return ret (which is 0) to signal success, as no further action (like loading the nf_log_syslog module) is needed.
576 } 577 request_module("%s", "nf_log_syslog"); 578 579 return ret; return 0.
It's 0 as well. Emm... do you know a way to make the Smatch static checker happy? Thanks, Lance
580 } regards, dan carpenter