Re: update check_cmn_err

Toomas Soome <[email protected]> Mon, 18 Nov 2024 13:32:52 +0200
Newsgroups org.kernel.vger.smatch
Message-ID <[email protected]>
Re-sending as plain text:D

Yes it does, thanks. Will get nice warning after adding something after =
ddi_err(DER_PANIC, ...:

=
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: ../../common/os/instance.c:1599 e_ddi_borrow_instance() warn: =
ignoring unreachable code.

thanks,
Toomas

> On 18. Nov 2024, at 12:42, Dan Carpenter <[email protected]> =
wrote:
>=20
> On Mon, Nov 18, 2024 at 10:07:42AM +0200, Toomas Soome wrote:
>>=20
>>=20
>>> On 18. Nov 2024, at 10:02, Dan Carpenter <[email protected]> =
wrote:
>>>=20
>>> On Mon, Nov 18, 2024 at 09:51:37AM +0200, Toomas Soome wrote:
>>>> Hi!
>>>>=20
>>>> I would like to update work done by John Levon, there is other =
function,
>>>> similar to cmn_err().
>>>>=20
>>>=20
>>> The cmn_err() function is a function in Illumos where if you pass =
CE_PANIC to
>>> it then it doesn't return.  Presumably if Smatch doesn't parse this =
correctly,
>>> then you end up with tons of uninitialized variable false positives. =
 Probably
>>> other false positives as well.
>>>=20
>>> Smatch is heavily tuned for the Linux kernel because that's where my =
focus has
>>> been for the past fifteen years.  Most of the easy parsing issues =
for the Linux
>>> kernel are already addressed.  Outside of the Linux kernel then =
Smatch is very
>>> untuned and quite bad.
>>>=20
>>> regards,
>>> dan carpenter
>>=20
>> Yep, this is for illumos, and since John did upstream the cmn_err() =
check, I
>> would like to complement it with ddi_err() as well;) We currently do =
have a
>> bit older version of smatch in use and I=E2=80=99m working to update =
it. Despite the
>> issues noted, it is still rather helpful of detecting problems;)
>=20
> Could you test this and let me know if it works for you?
>=20
> regards,
> dan carpenter
>=20
> diff --git a/check_cmn_err.c b/check_cmn_err.c
> index 1063efeb4774..ebdda365d7b9 100644
> --- a/check_cmn_err.c
> +++ b/check_cmn_err.c
> @@ -26,10 +26,11 @@
> #include "smatch.h"
> #include "smatch_extra.h"
>=20
> -#define CE_PANIC (3)
> +#define CE_PANIC (3)
> +#define DER_PANIC (7)
>=20
> void match_cmn_err(const char *fn, struct expression *expr,
> - void *unused)
> + void *panic_value)
> {
> struct expression *arg;
> sval_t sval;
> @@ -38,7 +39,7 @@ void match_cmn_err(const char *fn, struct expression =
*expr,
> if (!get_implied_value(arg, &sval))
> return;
>=20
> - if (sval.value =3D=3D CE_PANIC)
> + if (sval.value =3D=3D PTR_INT(panic_value))
> nullify_path();
> }
>=20
> @@ -48,5 +49,6 @@ void check_cmn_err(int id)
> if (option_project !=3D PROJ_ILLUMOS_KERNEL)
> return;
>=20
> - add_function_hook("cmn_err", &match_cmn_err, NULL);
> + add_function_hook("cmn_err", &match_cmn_err, INT_PTR(CE_PANIC));
> + add_function_hook("ddi_err", &match_cmn_err, INT_PTR(DER_PANIC));
> }