On Thu, Jul 22, 2021 at 12:01:41PM +0300, Amir Goldstein wrote: > On Thu, Jul 22, 2021 at 1:39 AM Matthew Bobrowski <repnop@xxxxxxxxxx> wrote: > > To make things clearer, avoid any future confusion and possibly tripping > > over such a bug, perhaps it'd be better to split up the fput(f) call into a > > separate branch outside of the current conditional, simply i.e. > > > > ... > > > > if (f) > > fput(f); > > > > ... > > > > Thoughts? > > smatch (apparently) does not know about the relation that f is non NULL > if (fd == FAN_NOFD) it needs to study create_fd() for that. > > I suggest to move fd_install(fd, f); right after checking of return value > from create_fd() and without the if (f) condition. > That should make it clear for human and robots reading this function > that the cleanup in out_close_fd label is correct. Yeah. I got "f" and "fd" mixed up in my head when I was reviewing this code. My bad. Smatch is *supposed* to know about the relationship between those two. The bug is actually that Smatch records in the database that create_fd() always fails. It's a serious bug, and I'm trying to investigate what's going on and I'm sure that I will fix this before the end of the week. No need to change the code just to work around a static checker bug. regards, dan carpenter
