| Message ID | 20200813175839.59422-6-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: SOF/Intel: fix cppcheck warnings | expand |
On Thu, Aug 13, 2020 at 12:58:39PM -0500, Pierre-Louis Bossart wrote: > - hdmi_num = sof_sdw_quirk & SOF_SDW_TGL_HDMI ? > + hdmi_num = (sof_sdw_quirk & SOF_SDW_TGL_HDMI) ? > SOF_TGL_HDMI_COUNT : SOF_PRE_TGL_HDMI_COUNT; Or better yet, just don't abuse the ternery operator like this and write normal conditional statements.
On 8/13/20 1:45 PM, Mark Brown wrote: > On Thu, Aug 13, 2020 at 12:58:39PM -0500, Pierre-Louis Bossart wrote: > >> - hdmi_num = sof_sdw_quirk & SOF_SDW_TGL_HDMI ? >> + hdmi_num = (sof_sdw_quirk & SOF_SDW_TGL_HDMI) ? >> SOF_TGL_HDMI_COUNT : SOF_PRE_TGL_HDMI_COUNT; > > Or better yet, just don't abuse the ternery operator like this and write > normal conditional statements. I count 795 uses of the ternary operator in sound/soc and 68776 in my local kernel branch. Can you clarify in what way this is an abuse? I don't mind changing this, I wasn't aware this is frowned upon.
On Thu, Aug 13, 2020 at 02:43:50PM -0500, Pierre-Louis Bossart wrote: > On 8/13/20 1:45 PM, Mark Brown wrote: > > On Thu, Aug 13, 2020 at 12:58:39PM -0500, Pierre-Louis Bossart wrote: > > > - hdmi_num = sof_sdw_quirk & SOF_SDW_TGL_HDMI ? > > > + hdmi_num = (sof_sdw_quirk & SOF_SDW_TGL_HDMI) ? > > > SOF_TGL_HDMI_COUNT : SOF_PRE_TGL_HDMI_COUNT; > > Or better yet, just don't abuse the ternery operator like this and write > > normal conditional statements. > I count 795 uses of the ternary operator in sound/soc and 68776 in my local > kernel branch. > Can you clarify in what way this is an abuse? I don't mind changing this, I > wasn't aware this is frowned upon. If you write a normal conditional statement then not only is the precedence clear but it's just generally easier to read. There are cases where it can help make things clearer (eg, avoiding the use of scratch variables to hold results) but this is most definitely not one of them and I don't understand everyone's enthusiasm for trying to put them in.
On 8/13/20 2:49 PM, Mark Brown wrote: > On Thu, Aug 13, 2020 at 02:43:50PM -0500, Pierre-Louis Bossart wrote: >> On 8/13/20 1:45 PM, Mark Brown wrote: >>> On Thu, Aug 13, 2020 at 12:58:39PM -0500, Pierre-Louis Bossart wrote: > >>>> - hdmi_num = sof_sdw_quirk & SOF_SDW_TGL_HDMI ? >>>> + hdmi_num = (sof_sdw_quirk & SOF_SDW_TGL_HDMI) ? >>>> SOF_TGL_HDMI_COUNT : SOF_PRE_TGL_HDMI_COUNT; > >>> Or better yet, just don't abuse the ternery operator like this and write >>> normal conditional statements. > >> I count 795 uses of the ternary operator in sound/soc and 68776 in my local >> kernel branch. >> Can you clarify in what way this is an abuse? I don't mind changing this, I >> wasn't aware this is frowned upon. > > If you write a normal conditional statement then not only is the > precedence clear but it's just generally easier to read. There are > cases where it can help make things clearer (eg, avoiding the use of > scratch variables to hold results) but this is most definitely not one > of them and I don't understand everyone's enthusiasm for trying to put > them in. That's fair, I am not a big fan either. Please drop this patch and we'll rework this machine driver. There's a set of updates planned anyways and we can add this cleanup in a separate set. Thanks!
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c index 2463d432bf4d..baff9e99f626 100644 --- a/sound/soc/intel/boards/sof_sdw.c +++ b/sound/soc/intel/boards/sof_sdw.c @@ -728,7 +728,7 @@ static int sof_card_dai_links_create(struct device *dev, for (i = 0; i < ARRAY_SIZE(codec_info_list); i++) codec_info_list[i].amp_num = 0; - hdmi_num = sof_sdw_quirk & SOF_SDW_TGL_HDMI ? + hdmi_num = (sof_sdw_quirk & SOF_SDW_TGL_HDMI) ? SOF_TGL_HDMI_COUNT : SOF_PRE_TGL_HDMI_COUNT; ssp_mask = SOF_SSP_GET_PORT(sof_sdw_quirk);
Fix cppcheck warning: sound/soc/intel/boards/sof_sdw.c:739:46: style: Clarify calculation precedence for '&' and '?'. [clarifyCalculation] hdmi_num = sof_sdw_quirk & SOF_SDW_TGL_HDMI ? ^ Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/intel/boards/sof_sdw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)