gbaudio_dapm_free_controls() iterates over widgets using the
list_for_each_entry*() family of macros from <linux/list.h>, which
leaves the loop cursor pointing to a meaningless structure if it
completes a traversal of the list. The cursor was set to NULL at the end
of the loop body, but would be overwritten by the final loop cursor
update.
Because of this behavior, the widget could be non-null after the loop
even if the widget wasn't found, and the cleanup logic would treat the
pointer as a valid widget to free.
To fix this, introduce a temporary variable to act as the loop cursor
and copy it to a variable that can be accessed after the loop finishes.
Due to not removing any list elements, use list_for_each_entry() instead
of list_for_each_entry_safe() in the revised loop.
This was detected with the help of Coccinelle.
Fixes: 510e340efe0c ("staging: greybus: audio: Add helper APIs for dynamic audio modules")
Cc: stable(a)vger.kernel.org
Reviewed-by: Dan Carpenter <dan.carpenter(a)oracle.com>
Reviewed-by: Johan Hovold <johan(a)kernel.org>
Signed-off-by: Jared Kangas <kangas.jd(a)gmail.com>
---
Changes since v1:
* Removed safe list iteration as suggested by Johan Hovold <johan(a)kernel.org>
* Updated patch changelog to explain the list iteration change
* Added tags to changelog based on feedback (Cc:, Fixes:, Reviewed-by:)
drivers/staging/greybus/audio_helper.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c
index 843760675876..05e91e6bc2a0 100644
--- a/drivers/staging/greybus/audio_helper.c
+++ b/drivers/staging/greybus/audio_helper.c
@@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
int num)
{
int i;
- struct snd_soc_dapm_widget *w, *next_w;
+ struct snd_soc_dapm_widget *w, *tmp_w;
#ifdef CONFIG_DEBUG_FS
struct dentry *parent = dapm->debugfs_dapm;
struct dentry *debugfs_w = NULL;
@@ -124,13 +124,13 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
mutex_lock(&dapm->card->dapm_mutex);
for (i = 0; i < num; i++) {
/* below logic can be optimized to identify widget pointer */
- list_for_each_entry_safe(w, next_w, &dapm->card->widgets,
- list) {
- if (w->dapm != dapm)
- continue;
- if (!strcmp(w->name, widget->name))
+ w = NULL;
+ list_for_each_entry(tmp_w, &dapm->card->widgets, list) {
+ if (tmp_w->dapm == dapm &&
+ !strcmp(tmp_w->name, widget->name)) {
+ w = tmp_w;
break;
- w = NULL;
+ }
}
if (!w) {
dev_err(dapm->dev, "%s: widget not found\n",
--
2.34.3
No entry is being removed from the list when iterating the widget list
in gbaudio_dapm_free_controls() so there's no need to use
list_for_each_entry_safe().
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/staging/greybus/audio_helper.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c
index 07461a5d97c7..05e91e6bc2a0 100644
--- a/drivers/staging/greybus/audio_helper.c
+++ b/drivers/staging/greybus/audio_helper.c
@@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
int num)
{
int i;
- struct snd_soc_dapm_widget *w, *next_w, *tmp_w;
+ struct snd_soc_dapm_widget *w, *tmp_w;
#ifdef CONFIG_DEBUG_FS
struct dentry *parent = dapm->debugfs_dapm;
struct dentry *debugfs_w = NULL;
@@ -125,8 +125,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
for (i = 0; i < num; i++) {
/* below logic can be optimized to identify widget pointer */
w = NULL;
- list_for_each_entry_safe(tmp_w, next_w, &dapm->card->widgets,
- list) {
+ list_for_each_entry(tmp_w, &dapm->card->widgets, list) {
if (tmp_w->dapm == dapm &&
!strcmp(tmp_w->name, widget->name)) {
w = tmp_w;
--
2.35.1
gbaudio_dapm_free_controls() iterates over widgets using
list_for_each_entry_safe(), which leaves the loop cursor pointing to a
meaningless structure if it completes a traversal of the list. The
cursor was set to NULL at the end of the loop body, but would be
overwritten by the final loop cursor update.
Because of this behavior, the widget could be non-null after the loop
even if the widget wasn't found, and the cleanup logic would treat the
pointer as a valid widget to free.
To fix this, introduce a temporary variable to act as the loop cursor
and copy it to a variable that can be accessed after the loop finishes.
This was detected with the help of Coccinelle.
Signed-off-by: Jared Kangas <kangas.jd(a)gmail.com>
---
drivers/staging/greybus/audio_helper.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c
index 843760675876..07461a5d97c7 100644
--- a/drivers/staging/greybus/audio_helper.c
+++ b/drivers/staging/greybus/audio_helper.c
@@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
int num)
{
int i;
- struct snd_soc_dapm_widget *w, *next_w;
+ struct snd_soc_dapm_widget *w, *next_w, *tmp_w;
#ifdef CONFIG_DEBUG_FS
struct dentry *parent = dapm->debugfs_dapm;
struct dentry *debugfs_w = NULL;
@@ -124,13 +124,14 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
mutex_lock(&dapm->card->dapm_mutex);
for (i = 0; i < num; i++) {
/* below logic can be optimized to identify widget pointer */
- list_for_each_entry_safe(w, next_w, &dapm->card->widgets,
+ w = NULL;
+ list_for_each_entry_safe(tmp_w, next_w, &dapm->card->widgets,
list) {
- if (w->dapm != dapm)
- continue;
- if (!strcmp(w->name, widget->name))
+ if (tmp_w->dapm == dapm &&
+ !strcmp(tmp_w->name, widget->name)) {
+ w = tmp_w;
break;
- w = NULL;
+ }
}
if (!w) {
dev_err(dapm->dev, "%s: widget not found\n",
--
2.34.3
On Mon, May 23, 2022 at 03:08:57PM +0300, Uri Arev wrote:
> Reported by Checkpatch:
> CHECK: Alignment should match open parenthesis
>
> -Uri
>
Don't put your name in the commit message like this.
> Signed-off-by: Uri Arev <me(a)wantyapps.xyz>
> ---
> v2: More understandable title, add Signed-off-by line
Why didn't you fix this one as well?
CHECK: Alignment should match open parenthesis
#452: FILE: drivers/staging/greybus/fw-management.c:452:
+ ret = fw_mgmt_load_and_validate_operation(fw_mgmt,
+ intf_load.load_method, intf_load.firmware_tag);
regards,
dan carpenter
On Sun, May 22, 2022 at 09:51:56PM +0300, Uri Arev wrote:
> Reported by Checkpatch:
> CHECK: Alignment should match open parenthesis
>
> - Uri
> ---
This not how you format a patch. Please run checkpatch.pl on your
patches. :P
regards,
dan carpenter
From: Arnd Bergmann <arnd(a)arndb.de>
During a patch discussion, Linus brought up the option of changing
the C standard version from gnu89 to gnu99, which allows using variable
declaration inside of a for() loop. While the C99, C11 and later standards
introduce many other features, most of these are already available in
gnu89 as GNU extensions as well.
An earlier attempt to do this when gcc-5 started defaulting to
-std=gnu11 failed because at the time that caused warnings about
designated initializers with older compilers. Now that gcc-5.1 is the
minimum compiler version used for building kernels, that is no longer a
concern. Similarly, the behavior of 'inline' functions changes between
gnu89 and gnu11, but this was taken care of by defining 'inline' to
include __attribute__((gnu_inline)) in order to allow building with
clang a while ago.
One minor issue that remains is an added gcc warning for shifts of
negative integers when building with -Werror, which happens with the
'make W=1' option, as well as for three drivers in the kernel that always
enable -Werror, but it was only observed with the i915 driver so far.
To be on the safe side, add -Wno-shift-negative-value to any -Wextra
in a Makefile.
Nathan Chancellor reported an additional -Wdeclaration-after-statement
warning that appears in a system header on arm, this still needs a
workaround.
The differences between gnu99, gnu11, gnu1x and gnu17 are fairly
minimal and mainly impact warnings at the -Wpedantic level that the
kernel never enables. Between these, gnu11 is the newest version
that is supported by all supported compiler versions, though it is
only the default on gcc-5, while all other supported versions of
gcc or clang default to gnu1x/gnu17.
Link: https://lore.kernel.org/lkml/CAHk-=wiyCH7xeHcmiFJ-YgXUy2Jaj7pnkdKpcovt8fYbV…
Link: https://github.com/ClangBuiltLinux/linux/issues/1603
Suggested-by: Linus Torvalds <torvalds(a)linux-foundation.org>
Cc: Masahiro Yamada <masahiroy(a)kernel.org>
Cc: linux-kbuild(a)vger.kernel.org
Cc: llvm(a)lists.linux.dev
Signed-off-by: Arnd Bergmann <arnd(a)arndb.de>
---
[v2]
- added -std=gnu11 back, rather than just relying on the default
- minor changes to changelog text
---
Documentation/process/programming-language.rst | 4 ++--
.../translations/it_IT/process/programming-language.rst | 4 ++--
.../translations/zh_CN/process/programming-language.rst | 4 ++--
.../translations/zh_TW/process/programming-language.rst | 4 ++--
Makefile | 6 +++---
arch/arm64/kernel/vdso32/Makefile | 2 +-
drivers/gpu/drm/i915/Makefile | 1 +
drivers/staging/greybus/tools/Makefile | 3 ++-
fs/btrfs/Makefile | 1 +
scripts/Makefile.extrawarn | 1 +
10 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/Documentation/process/programming-language.rst b/Documentation/process/programming-language.rst
index ec474a70a02f..894f2a6eb9db 100644
--- a/Documentation/process/programming-language.rst
+++ b/Documentation/process/programming-language.rst
@@ -5,8 +5,8 @@ Programming Language
The kernel is written in the C programming language [c-language]_.
More precisely, the kernel is typically compiled with ``gcc`` [gcc]_
-under ``-std=gnu89`` [gcc-c-dialect-options]_: the GNU dialect of ISO C90
-(including some C99 features). ``clang`` [clang]_ is also supported, see
+under ``-std=gnu11`` [gcc-c-dialect-options]_: the GNU dialect of ISO C11
+(including some C17 features). ``clang`` [clang]_ is also supported, see
docs on :ref:`Building Linux with Clang/LLVM <kbuild_llvm>`.
This dialect contains many extensions to the language [gnu-extensions]_,
diff --git a/Documentation/translations/it_IT/process/programming-language.rst b/Documentation/translations/it_IT/process/programming-language.rst
index 41db2598ce11..aa21097737ae 100644
--- a/Documentation/translations/it_IT/process/programming-language.rst
+++ b/Documentation/translations/it_IT/process/programming-language.rst
@@ -10,8 +10,8 @@ Linguaggio di programmazione
Il kernel è scritto nel linguaggio di programmazione C [it-c-language]_.
Più precisamente, il kernel viene compilato con ``gcc`` [it-gcc]_ usando
-l'opzione ``-std=gnu89`` [it-gcc-c-dialect-options]_: il dialetto GNU
-dello standard ISO C90 (con l'aggiunta di alcune funzionalità da C99).
+l'opzione ``-std=gnu11`` [it-gcc-c-dialect-options]_: il dialetto GNU
+dello standard ISO C11 (con l'aggiunta di alcune funzionalità da C17).
Linux supporta anche ``clang`` [it-clang]_, leggete la documentazione
:ref:`Building Linux with Clang/LLVM <kbuild_llvm>`.
diff --git a/Documentation/translations/zh_CN/process/programming-language.rst b/Documentation/translations/zh_CN/process/programming-language.rst
index 2a47a1d2ec20..58d2b3bd2d85 100644
--- a/Documentation/translations/zh_CN/process/programming-language.rst
+++ b/Documentation/translations/zh_CN/process/programming-language.rst
@@ -9,8 +9,8 @@
============
内核是用C语言 :ref:`c-language <cn_c-language>` 编写的。更准确地说，内核通常是用 :ref:`gcc <cn_gcc>`
-在 ``-std=gnu89`` :ref:`gcc-c-dialect-options <cn_gcc-c-dialect-options>` 下编译的：ISO C90的 GNU 方言（
-包括一些C99特性）
+在 ``-std=gnu11`` :ref:`gcc-c-dialect-options <cn_gcc-c-dialect-options>` 下编译的：ISO C11的 GNU 方言（
+包括一些C17特性）
这种方言包含对语言 :ref:`gnu-extensions <cn_gnu-extensions>` 的许多扩展，当然，它们许多都在内核中使用。
diff --git a/Documentation/translations/zh_TW/process/programming-language.rst b/Documentation/translations/zh_TW/process/programming-language.rst
index 54e3699eadf8..235de05f7e2c 100644
--- a/Documentation/translations/zh_TW/process/programming-language.rst
+++ b/Documentation/translations/zh_TW/process/programming-language.rst
@@ -12,8 +12,8 @@
============
內核是用C語言 :ref:`c-language <tw_c-language>` 編寫的。更準確地說，內核通常是用 :ref:`gcc <tw_gcc>`
-在 ``-std=gnu89`` :ref:`gcc-c-dialect-options <tw_gcc-c-dialect-options>` 下編譯的：ISO C90的 GNU 方言（
-包括一些C99特性）
+在 ``-std=gnu11`` :ref:`gcc-c-dialect-options <tw_gcc-c-dialect-options>` 下編譯的：ISO C11的 GNU 方言（
+包括一些C17特性）
這種方言包含對語言 :ref:`gnu-extensions <tw_gnu-extensions>` 的許多擴展，當然，它們許多都在內核中使用。
diff --git a/Makefile b/Makefile
index 289ce2be8032..66496eaeb9ec 100644
--- a/Makefile
+++ b/Makefile
@@ -432,7 +432,7 @@ HOSTCXX = g++
endif
export KBUILD_USERCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes \
- -O2 -fomit-frame-pointer -std=gnu89
+ -O2 -fomit-frame-pointer -std=gnu11
export KBUILD_USERLDFLAGS :=
KBUILD_HOSTCFLAGS := $(KBUILD_USERCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS)
@@ -515,7 +515,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \
-Werror=implicit-function-declaration -Werror=implicit-int \
-Werror=return-type -Wno-format-security \
- -std=gnu89
+ -std=gnu11
KBUILD_CPPFLAGS := -D__KERNEL__
KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
@@ -782,7 +782,7 @@ KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
ifdef CONFIG_CC_IS_CLANG
KBUILD_CPPFLAGS += -Qunused-arguments
-# The kernel builds with '-std=gnu89' so use of GNU extensions is acceptable.
+# The kernel builds with '-std=gnu11' so use of GNU extensions is acceptable.
KBUILD_CFLAGS += -Wno-gnu
# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the
# source of a reference will be _MergedGlobals and not on of the whitelisted names.
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 6c01b63ff56d..9378ea055bf2 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -68,7 +68,7 @@ VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common \
-Werror-implicit-function-declaration \
-Wno-format-security \
- -std=gnu89
+ -std=gnu11
VDSO_CFLAGS += -O2
# Some useful compiler-dependent flags from top-level Makefile
VDSO_CFLAGS += $(call cc32-option,-Wdeclaration-after-statement,)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1b62b9f65196..1618a6e0af4e 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -17,6 +17,7 @@ subdir-ccflags-y += -Wno-unused-parameter
subdir-ccflags-y += -Wno-type-limits
subdir-ccflags-y += -Wno-missing-field-initializers
subdir-ccflags-y += -Wno-sign-compare
+subdir-ccflags-y += -Wno-shift-negative-value
subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
diff --git a/drivers/staging/greybus/tools/Makefile b/drivers/staging/greybus/tools/Makefile
index ad0ae8053b79..a3bbd73171f2 100644
--- a/drivers/staging/greybus/tools/Makefile
+++ b/drivers/staging/greybus/tools/Makefile
@@ -12,7 +12,8 @@ CFLAGS += -std=gnu99 -Wall -Wextra -g \
-Wredundant-decls \
-Wcast-align \
-Wsign-compare \
- -Wno-missing-field-initializers
+ -Wno-missing-field-initializers \
+ -Wno-shift-negative-value
CC := $(CROSS_COMPILE)gcc
diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 4188ba3fd8c3..99f9995670ea 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -17,6 +17,7 @@ subdir-ccflags-y += $(condflags)
subdir-ccflags-y += -Wno-missing-field-initializers
subdir-ccflags-y += -Wno-sign-compare
subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-shift-negative-value
obj-$(CONFIG_BTRFS_FS) := btrfs.o
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 8be892887d71..650d0b8ceec3 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -36,6 +36,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
KBUILD_CFLAGS += -Wno-missing-field-initializers
KBUILD_CFLAGS += -Wno-sign-compare
KBUILD_CFLAGS += -Wno-type-limits
+KBUILD_CFLAGS += -Wno-shift-negative-value
KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN1
--
2.29.2
Correct misspelling in comment. Issue found with checkpatch.
Signed-off-by: Nickalas Cason <nickalas(a)cason.dev>
---
drivers/staging/greybus/arche-platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index e374dfc0c92f..fcbd5f71eff2 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -591,7 +591,7 @@ static __maybe_unused int arche_platform_suspend(struct device *dev)
static __maybe_unused int arche_platform_resume(struct device *dev)
{
/*
- * Atleast for ES2 we have to meet the delay requirement between
+ * At least for ES2 we have to meet the delay requirement between
* unipro switch and AP bridge init, depending on whether bridge is in
* OFF state or standby state.
*
--
2.35.1
