Skip to content

ASoC: SOF: topology: goto found: if a dai is found - #392

Merged
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
bardliao:issue-343
Dec 10, 2018
Merged

ASoC: SOF: topology: goto found: if a dai is found#392
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
bardliao:issue-343

Conversation

@bardliao

@bardliao bardliao commented Dec 7, 2018 •
edited
Loading

Copy link
Copy Markdown
Collaborator

sof_dai will be assigned when list_for_each_entry walk through list.
So, sof_dai won't be null even there is no dai found. Use goto statement
instead.

Signed-off-by: Bard liao yung-chuan.liao@linux.intel.com

ranj063
ranj063 reviewed Dec 7, 2018
View reviewed changes
Comment thread sound/soc/sof/topology.c Outdated

@ranj063 ranj063 Dec 7, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bardliao does this really fix the problem? sof_dai will be assigned as the list is iterated so I dont see the use of a temp variable.

@keyonjie keyonjie Dec 7, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 the issue is in line 2395, we were using sof_dai as a matching flag at the same time, I think this is a good fix.

@ranj063 ranj063 Dec 7, 2018 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bardliao @keyonjie i see the problem now, Thanks!. I think we can avoid the temp variable with a goto statement instead that points to the switch below.

@bardliao bardliao Dec 7, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 This PR is related to #343 . sof_link_unload() could be called with dai_link which is not in the dai_list. We won't find a dai in such case. And we use "if (!sof_dai)" in line 2395 to detect if we find a dai. But it won't be true even we can't find a dai.

@bardliao bardliao Dec 7, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 good idea. PR updated.

@bardliao bardliao changed the title ASoC: SOF: topology: fix wrong judgment of finding dai ASoC: SOF: topology: goto find: if a dai is found Dec 7, 2018
ranj063
ranj063 reviewed Dec 7, 2018
View reviewed changes
Comment thread sound/soc/sof/topology.c Outdated

@ranj063 ranj063 Dec 7, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bardliao sorry for the nitpick :) but can this be "found" instead?

@bardliao bardliao Dec 7, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 of course :)

@bardliao
bardliao force-pushed the issue-343 branch 2 times, most recently from 09329db to e09f08c Compare December 7, 2018 06:01
@bardliao bardliao changed the title ASoC: SOF: topology: goto find: if a dai is found ASoC: SOF: topology: goto found: if a dai is found Dec 7, 2018
plbossart
plbossart approved these changes Dec 7, 2018
View reviewed changes

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please try and resolve conflicts before submitting next time, github lets you know if you have any conflicts in the PR.

@plbossart

plbossart commented Dec 7, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

well the conflict resolution failed, so please resubmit.

@bardliao
ASoC: SOF: topology: goto found: if a dai is found
fcdc49a 
sof_dai will be assigned when list_for_each_entry walk through list.
So, sof_dai won't be null even there is no dai found. Use goto statement
instead.

Signed-off-by: Bard liao <yung-chuan.liao@linux.intel.com>
@bardliao
bardliao force-pushed the issue-343 branch 2 times, most recently from e09f08c to fcdc49a Compare December 8, 2018 08:38
@bardliao

bardliao commented Dec 8, 2018

Copy link
Copy Markdown
Collaborator Author

@plbossart The conflict is resolved. thanks

plbossart
plbossart reviewed Dec 10, 2018
View reviewed changes

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bardliao the ask was to keep the error: prefix. I'll merge and fix this myself.

@plbossart
plbossart merged commit 7923fc5 into thesofproject:topic/sof-dev Dec 10, 2018
@bardliao

bardliao commented Dec 11, 2018

Copy link
Copy Markdown
Collaborator Author

@plbossart Thanks :)

@bardliao
bardliao deleted the issue-343 branch December 15, 2018 07:45
@mengdonglin mengdonglin mentioned this pull request Dec 29, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossart plbossart plbossart approved these changes
@ranj063 ranj063 ranj063 approved these changes
@RanderWang RanderWang Awaiting requested review from RanderWang
+1 more reviewer
@keyonjie keyonjie keyonjie left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bardliao @plbossart @ranj063 @keyonjie