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/smatch: ../../common/os/instance.c:1599 e_ddi_borrow_instance() warn: ignoring unreachable code.
thanks,
Toomas
> On 18. Nov 2024, at 12:42, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> On Mon, Nov 18, 2024 at 10:07:42AM +0200, Toomas Soome wrote:
>>
>>
>>> On 18. Nov 2024, at 10:02, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>>>
>>> On Mon, Nov 18, 2024 at 09:51:37AM +0200, Toomas Soome wrote:
>>>> Hi!
>>>>
>>>> I would like to update work done by John Levon, there is other function,
>>>> similar to cmn_err().
>>>>
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> regards,
>>> dan carpenter
>>
>> 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’m working to update it. Despite the
>> issues noted, it is still rather helpful of detecting problems;)
>
> Could you test this and let me know if it works for you?
>
> regards,
> dan carpenter
>
> 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"
>
> -#define CE_PANIC (3)
> +#define CE_PANIC (3)
> +#define DER_PANIC (7)
>
> 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;
>
> - if (sval.value == CE_PANIC)
> + if (sval.value == PTR_INT(panic_value))
> nullify_path();
> }
>
> @@ -48,5 +49,6 @@ void check_cmn_err(int id)
> if (option_project != PROJ_ILLUMOS_KERNEL)
> return;
>
> - 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));
> }
