Tony Chuang <[email protected]> writes: >> Subject: Re: [PATCH 5/6] rtw88: mac: remove dangerous while (1) >> >> <[email protected]> writes: >> >> > From: Yan-Hsuan Chuang <[email protected]> >> > >> > Not to use while (1) to parse power sequence commands in an array. >> > Put the statement (when cmd is not NULL) instead to make the loop stop >> > when the next fetched command is NULL. >> > >> > Signed-off-by: Yan-Hsuan Chuang <[email protected]> >> > --- >> > drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------ >> > 1 file changed, 3 insertions(+), 6 deletions(-) >> > >> > diff --git a/drivers/net/wireless/realtek/rtw88/mac.c >> b/drivers/net/wireless/realtek/rtw88/mac.c >> > index 25a923b..7487b2e 100644 >> > --- a/drivers/net/wireless/realtek/rtw88/mac.c >> > +++ b/drivers/net/wireless/realtek/rtw88/mac.c >> > @@ -203,17 +203,14 @@ static int rtw_pwr_seq_parser(struct rtw_dev >> *rtwdev, >> > return -EINVAL; >> > } >> > >> > - do { >> > - cmd = cmd_seq[idx]; >> > - if (!cmd) >> > - break; >> > - >> > + while ((cmd = cmd_seq[idx])) { >> > ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd); >> > if (ret) >> > return -EBUSY; >> > >> > + /* fetch next command */ >> > idx++; >> > - } while (1); >> > + }; >> >> I dount see how this is any better, with a suitable bug you can still >> have a neverending loop, right? I was thinking more something like this: >> >> count = 100; >> do { >> .... >> } while (count--); >> >> That way the won't be more than 100 loops no matter how many bugs there >> are :) Of course I have no idea what would be a good value for count. >> > > To make this totally safe, I think we need to re-write the power seq parsing > code. > I think I should drop this patch, and write a better code later. > > And also re-write the polling command, to remove the while (1).
Sounds good to me. -- Kalle Valo
