On Tue, Jun 18, 2024 at 02:00:52PM -0700, Badhri Jagan Sridharan wrote:
> On Mon, Jun 17, 2024 at 6:30 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >
> > Hello Badhri Jagan Sridharan,
> >
> > Commit 599f008c257d ("usb: typec: tcpm: Add callbacks to mitigate
> > wakeups due to contaminant") from Jan 14, 2023 (linux-next), leads to
> > the following Smatch static checker warning:
> >
> > drivers/usb/typec/tcpm/tcpm.c:4620 run_state_machine()
> > error: we previously assumed 'port->tcpc->check_contaminant' could be null (see line 4607)
> >
> > drivers/usb/typec/tcpm/tcpm.c
> > 4600 static void run_state_machine(struct tcpm_port *port)
> > 4601 {
> > 4602 int ret;
> > 4603 enum typec_pwr_opmode opmode;
> > 4604 unsigned int msecs;
> > 4605 enum tcpm_state upcoming_state;
> > 4606
> > 4607 if (port->tcpc->check_contaminant && port->state != CHECK_CONTAMINANT)
> >
> > Assume that ->check_contaminant is NULL and port->state == CHECK_CONTAMINANT
> >
> > 4608 port->potential_contaminant = ((port->enter_state == SRC_ATTACH_WAIT &&
> > 4609 port->state == SRC_UNATTACHED) ||
> > 4610 (port->enter_state == SNK_ATTACH_WAIT &&
> > 4611 port->state == SNK_UNATTACHED) ||
> > 4612 (port->enter_state == SNK_DEBOUNCED &&
> > 4613 port->state == SNK_UNATTACHED));
> > 4614
> > 4615 port->enter_state = port->state;
> > 4616 switch (port->state) {
> > 4617 case TOGGLING:
> > 4618 break;
> > 4619 case CHECK_CONTAMINANT:
> > --> 4620 port->tcpc->check_contaminant(port->tcpc);
>
>
> Hi Dan,
>
> We could add a null pointer check before invoking
> port->tcpc->check_contaminant(), however, it would be redundant.
> Unless port->potential_contaminant is set, TCPM would never call
> tcpm_set_state(port, CHECK_CONTAMINANT..) and
> port->potential_contaminant would not be set unless
> port->tcpc->check_contaminant () is NOT NULL.
>
> So the assumption of "->check_contaminant is NULL and port->state ==
> CHECK_CONTAMINANT" is not possible in the code as it stands. But we
> can add an explicit check for port->tcpc->check_contaminant() in
> CHECK_CONTAMINANT to make it more obvious if needed.
Nah. Forget about it. When it's a false positive, just ignore it.
If anyone has questions later they can find this thread.
regards,
dan carpenter
