Op 21-10-2024 om 09:54 schreef Dan Carpenter:
On Sun, Oct 20, 2024 at 09:21:43PM +0200, Kees Bakker wrote:
This was detected by Coverity, CID 1600787
Signed-off-by: Kees Bakker <kees@xxxxxxxxxxxx>
This is how your patch looks like in lore.
https://lore.kernel.org/all/20241020192243.C6A0F18DA7F@xxxxxxxxxxxxxxxxxx/
The subject and the body of the email are far apart. It's like a book, right?
The title of the book isn't the first sentence of the book. Feel free to
restate the subject if necessary. The commit message should really say the
impact of the bug, (how it looks like to the user) and you need a Fixes tag.
---
drivers/staging/gpib/common/gpib_os.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/gpib/common/gpib_os.c b/drivers/staging/gpib/common/gpib_os.c
index d5860a0a131f..57ff1ed30dac 100644
--- a/drivers/staging/gpib/common/gpib_os.c
+++ b/drivers/staging/gpib/common/gpib_os.c
@@ -1667,6 +1667,8 @@ static int autospoll_ioctl(gpib_board_t *board, gpib_file_private_t *file_priv,
return -EFAULT;
desc = handle_to_descriptor(file_priv, 0); /* board handle is 0 */
+ if (!desc)
+ return -EINVAL;
There is always a board 0 so the check is unnecessary. This kind of heuristic
tends to have a lot of false positives. It's better to just ignore static
checkers when they are wrong.
Let me go ahead and disagree with you there.
Notice that we already have the following code in ibclose()
desc = handle_to_descriptor(priv, 0);
if (desc) {
So my change isn't that alien in this module.
If we really hate to add that check, then there can be two alternatives
1. add a comment
2. change the code as follows:
/* There is always a board 0 */
desc = file_priv->descriptors[0];
regards,
dan carpenter
