From: Dan Carpenter <error27@gmail.com>
To: Andrei Khomenkov <khomenkov@mailbox.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-staging@lists.linux.dev
Subject: Re: [PATCH 1/2] staging: rtl8723bs: remove dead rtw_atimdone_event_callback()
Date: Sat, 23 May 2026 13:32:28 +0300 [thread overview]
Message-ID: <ahGCPHzSRKys3gpp@stanley.mountain> (raw)
In-Reply-To: <20260522190256.49830-2-khomenkov@mailbox.org>
On Fri, May 22, 2026 at 10:02:55PM +0300, Andrei Khomenkov wrote:
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index a86d6f97cf02..c56082cd5264 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -5619,7 +5619,7 @@ static struct fwevent wlanevents[] = {
> {0, &rtw_joinbss_event_callback}, /*10*/
> {sizeof(struct stassoc_event), &rtw_stassoc_event_callback},
> {sizeof(struct stadel_event), &rtw_stadel_event_callback},
> - {0, &rtw_atimdone_event_callback},
> + {0, rtw_dummy_event_callback},
> {0, rtw_dummy_event_callback},
> {0, NULL}, /*15*/
> {0, NULL},
This patch is fine...
Reviewed-by: Dan Carpenter <error27@gmail.com>
But, dummy functions are really discouraged. It's supposed to be that
the caller checks for NULL and then doesn't call it. No point in having
a do nothing dummy function when we could just store a NULL pointer there
instead.
And most of that table is NULL pointers! I looked at the callers and
it's alarming that I don't immediately see any NULL checks.
5657 /* checking if event size match the event parm size */
5658 if ((wlanevents[evt_code].parmsize != 0) &&
If the callback is NULL then parmsize is 0 so we don't care about the
size... Just continue.
5659 (wlanevents[evt_code].parmsize != evt_sz))
5660 goto _abort_event_;
5661
5662 atomic_inc(&pevt_priv->event_seq);
5663
5664 peventbuf += 2;
5665
5666 if (peventbuf) {
5667 event_callback = wlanevents[evt_code].event_callback;
Get the call back.
5668 event_callback(padapter, (u8 *)peventbuf);
And crash... I guess we are relying on this table to be in sync with
the wlancmds[] table in drivers/staging/rtl8723bs/core/rtw_cmd.c.
That seems pretty risky to me.
5669
5670 pevt_priv->evt_done_cnt++;
5671 }
regards,
dan carpenter
next prev parent reply other threads:[~2026-05-23 10:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-22 19:02 [PATCH 0/2] staging: rtl8723bs: remove dead function and clean up table Andrei Khomenkov
2026-05-22 19:02 ` [PATCH 1/2] staging: rtl8723bs: remove dead rtw_atimdone_event_callback() Andrei Khomenkov
2026-05-23 10:32 ` Dan Carpenter [this message]
2026-05-22 19:02 ` [PATCH 2/2] staging: rtl8723bs: clean up wlanevents table in rtw_mlme_ext.c Andrei Khomenkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ahGCPHzSRKys3gpp@stanley.mountain \
--to=error27@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=khomenkov@mailbox.org \
--cc=linux-staging@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.