Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jim Cromie <jim.cromie@gmail.com>
To: daniel.vetter@ffwll.ch, daniel@ffwll.ch, jbaron@akamai.com,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
Cc: apw@canonical.com, jani.nikula@intel.com,
	linux@rasmusvillemoes.dk, Jim Cromie <jim.cromie@gmail.com>,
	seanpaul@chromium.org, joe@perches.com
Subject: [Intel-gfx] [PATCH v5 13/22] checkpatch: file-scoped extern special case for linker-symbol
Date: Tue,  1 Aug 2023 11:02:45 -0600	[thread overview]
Message-ID: <20230801170255.163237-14-jim.cromie@gmail.com> (raw)
In-Reply-To: <20230801170255.163237-1-jim.cromie@gmail.com>

"externs should be avoided in .c files" needs an exception for linker
symbols, like those that mark the start, stop of many kernel sections.

Since code already checks REALNAME to avoid linker-scripts entirely,
add a new else-if block to look at them instead.  As a simple
heuristic, treat all words (in the patch-line) as possible symbols,
and save them to screen the WARN quoted above.  For my test case, this
included BOUNDED_BY (a macro), which is extra, but not troublesome.

Where the WARN is issued, precede it with an else-if block to catch
one common extern-in-c use case: "extern struct foo bar[]".  Here we
can at least issue a softer warning, after checking for a match with a
maybe-linker-symbol parsed earlier from the patch.  Though heuristic,
it worked for my test-case, allowing both (start|stop)$symbol's,
matched by $symbol, it wasn't thrown by noise (BOUNDED_BY) in the
maybe-linker-symbols.

NB: git diff ordering dependence on vmlinux.lds.h before c files ?

Cc: apw@canonical.com
Cc: joe@perches.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 scripts/checkpatch.pl | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b30114d637c4..f6b6b2a50dfe 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -74,6 +74,8 @@ my $git_command ='export LANGUAGE=en_US.UTF-8; git';
 my $tabsize = 8;
 my ${CONFIG_} = "CONFIG_";
 
+my %maybe_linker_symbol; # for externs in c exceptions, when seen in *vmlinux.lds.h
+
 sub help {
 	my ($exitcode) = @_;
 
@@ -6051,6 +6053,9 @@ sub process {
 
 # check for line continuations outside of #defines, preprocessor #, and asm
 
+		} elsif ($realfile =~ m@/vmlinux.lds.h$@) {
+		    $line =~ s/(\w+)/$maybe_linker_symbol{$1}++/ge;
+		    #print "REAL: $realfile\nln: $line\nkeys:", sort keys %maybe_linker_symbol;
 		} else {
 			if ($prevline !~ /^..*\\$/ &&
 			    $line !~ /^\+\s*\#.*\\$/ &&		# preprocessor
@@ -7107,6 +7112,21 @@ sub process {
 				     "arguments for function declarations should follow identifier\n" . $herecurr);
 			}
 
+		} elsif ($realfile =~ /\.c$/ && defined $stat &&
+		    $stat =~ /^\+extern struct\s+(\w+)\s+(\w+)\[\];/)
+		{
+			my ($st_type, $st_name) = ($1, $2);
+
+			for my $s (keys %maybe_linker_symbol) {
+			    #print "Linker symbol? $st_name : $s\n";
+			    goto LIKELY_LINKER_SYMBOL
+				if $st_name =~ /$s/;
+			}
+			WARN("AVOID_EXTERNS",
+			     "found a file-scoped extern type:$st_type name:$st_name in .c file\n"
+			     . "is this a linker symbol ?\n" . $herecurr);
+		  LIKELY_LINKER_SYMBOL:
+
 		} elsif ($realfile =~ /\.c$/ && defined $stat &&
 		    $stat =~ /^.\s*extern\s+/)
 		{
-- 
2.41.0


  parent reply	other threads:[~2023-08-01 17:03 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-01 17:02 [Intel-gfx] [PATCH v5 00/22] fix DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 01/22] drm: use correct ccflags-y syntax Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 02/22] test-dyndbg: fixup CLASSMAP usage error Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 03/22] dyndbg: make ddebug_class_param union members same size Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 04/22] dyndbg: replace classmap list with a vector Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 05/22] dyndbg: ddebug_apply_class_bitmap - add module arg, select on it Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 06/22] dyndbg: split param_set_dyndbg_classes to module/wrapper fns Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 07/22] dyndbg: drop NUM_TYPE_ARRAY Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 08/22] dyndbg: reduce verbose/debug clutter Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 09/22] dyndbg: silence debugs with no-change updates Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 10/22] dyndbg: tighten ddebug_class_name() 1st arg type Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 11/22] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 12/22] dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code Jim Cromie
2023-08-01 17:02 ` Jim Cromie [this message]
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 13/22] checkpatch: special case for file-scoped extern linker-symbol Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 14/22] dyndbg-API: fix CONFIG_DRM_USE_DYNAMIC_DEBUG regression Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 15/22] dyndbg: add for_each_boxed_vector Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 16/22] dyndbg: refactor ddebug_classparam_clamp_input Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 17/22] dyndbg-API: promote DYNDBG_CLASSMAP_PARAM to API Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 18/22] dyndbg-test: build it with just CONFIG_DYNAMIC_DEBUG_CORE Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 19/22] drm: restore CONFIG_DRM_USE_DYNAMIC_DEBUG un-BROKEN Jim Cromie
2023-08-03  7:13   ` kernel test robot
2023-08-03 20:04     ` jim.cromie
2023-08-03 20:21     ` jim.cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 20/22] drm-drivers: DRM_CLASSMAP_USE in 2nd batch of drivers, helpers Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 21/22] dyndbg-doc: add classmap info to howto Jim Cromie
2023-08-01 17:02 ` [Intel-gfx] [PATCH v5 22/22] checkpatch: reword long-line warn about commit-msg Jim Cromie
2023-08-01 17:57 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fix DRM_USE_DYNAMIC_DEBUG regression (rev3) Patchwork
2023-08-01 17:57 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230801170255.163237-14-jim.cromie@gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=apw@canonical.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=jbaron@akamai.com \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=seanpaul@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox