Re: checkpatch not warning for some split strings

From: Josh Triplett
Date: Mon Dec 02 2013 - 16:34:14 EST


On Mon, Dec 02, 2013 at 01:23:58PM -0800, Joe Perches wrote:
> Hey Josh.
>
> Back when you added commit ca56dc098ca
> ("checkpatch: check for quoted strings broken across lines")
>
> You added this test case:
>
> Test case:
>
> void context(void)
> {
> struct { unsigned magic; const char *strdata; } foo[] = {
> { 42, "these strings"
> "do not produce warnings" },
> { 256, "though perhaps"
> "they should" },
> };
>
> which avoids emitting a warning on split strings across
> multiple lines without a function-call like '(' use on
> the line above.
>
> Perhaps the "\(" test should be removed as checkpatch
> doesn't emit a warning for some uses like:
>
> diff --git a/drivers/net/wireless/iwlwifi/mvm/rs.c b/drivers/net/wireless/iwlwifi/mvm/rs.c
> []
> @@ -1868,7 +1865,8 @@ static void rs_rate_scale_perform(struct iwl_mvm *mvm,
> /* Else poor success; go back to mode in "active" table */
> } else {
> IWL_DEBUG_RATE(mvm,
> - "LQ: GOING BACK TO THE OLD TABLE suc=%d cur-tpt=%d old-tpt=%d\n",
> + "GOING BACK TO THE OLD TABLE: SR %d "
> + "cur-tpt %d old-tpt %d\n",
> window->success_ratio,
> window->average_tpt,
> lq_sta->last_tpt);

That one should definitely emit a warning. However, removing the \(
test entirely will introduce warnings on many other strings embedded in
data structures; only drop that test if you've diffed the checkpatch
output for the kernel with and without it and you're comfortable with
the *huge* number of additional warnings it'll introduce.

- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/