Add basic tests to cover some regressions that we had.
It's hard to test floppy because some tests require
presence or absense of a diskette in a drive. To simulate
test conditions and automate the testing I added
"run_*.sh" wrapper scripts that run tests in QEMU.
The first patch just improves check for reverted commits
in a commit message. The second patch is required to
generate a minimal initrd used in next commits. Rest of
commits are basic floppy tests.
Please, comment the approach, selftests integration
and suggest tests that you would like to add.
I thought about adding a possibility to remove/insert
diskettes inside a test. This is possible if we give
the guest an access to the QEMU monitor (eject/change cmds).
But I didn't find a better way to do it than to map a
monitor to an external port:
-monitor tcp:<ip>:<port>,server,nowait
and access this ip from the guest.
Maybe it's also possible to do with virtserialport.
Denis Efremov (5):
checkpatch: improve handling of revert commits
gen_initramfs.sh: use absolute path for gen_init_cpio
selftests: floppy: add basic tests for opening an empty device
selftests: floppy: add basic tests for a readonly disk
selftests: floppy: add basic rdwr tests
MAINTAINERS | 1 +
scripts/checkpatch.pl | 12 +--
tools/testing/selftests/floppy/.gitignore | 8 ++
tools/testing/selftests/floppy/Makefile | 10 ++
tools/testing/selftests/floppy/config | 1 +
tools/testing/selftests/floppy/empty.c | 58 ++++++++++++
tools/testing/selftests/floppy/init.c | 43 +++++++++
tools/testing/selftests/floppy/lib.sh | 67 +++++++++++++
tools/testing/selftests/floppy/rdonly.c | 99 ++++++++++++++++++++
tools/testing/selftests/floppy/rdwr.c | 67 +++++++++++++
tools/testing/selftests/floppy/run_empty.sh | 16 ++++
tools/testing/selftests/floppy/run_rdonly.sh | 22 +++++
tools/testing/selftests/floppy/run_rdwr.sh | 22 +++++
usr/gen_initramfs.sh | 2 +-
14 files changed, 421 insertions(+), 7 deletions(-)
create mode 100644 tools/testing/selftests/floppy/.gitignore
create mode 100644 tools/testing/selftests/floppy/Makefile
create mode 100644 tools/testing/selftests/floppy/config
create mode 100644 tools/testing/selftests/floppy/empty.c
create mode 100644 tools/testing/selftests/floppy/init.c
create mode 100644 tools/testing/selftests/floppy/lib.sh
create mode 100644 tools/testing/selftests/floppy/rdonly.c
create mode 100644 tools/testing/selftests/floppy/rdwr.c
create mode 100755 tools/testing/selftests/floppy/run_empty.sh
create mode 100755 tools/testing/selftests/floppy/run_rdonly.sh
create mode 100755 tools/testing/selftests/floppy/run_rdwr.sh
--
2.31.1
Use absolute path to call gen_init_cpio. This allows one
to use gen_initramfs.sh from any directory.
Cc: Masahiro Yamada <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
usr/gen_initramfs.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
index 63476bb70b41..2e4a86181c79 100755
--- a/usr/gen_initramfs.sh
+++ b/usr/gen_initramfs.sh
@@ -244,4 +244,4 @@ if test -n "$KBUILD_BUILD_TIMESTAMP"; then
timestamp="-t $timestamp"
fi
fi
-usr/gen_init_cpio $timestamp $cpio_list > $output
+"$(dirname "$0")"/gen_init_cpio $timestamp $cpio_list > $output
--
2.31.1
Add basic tests for O_NDELAY and O_ACCMODE open of an empty floppy device.
"empty" test works under assumption that there is no disk in "/dev/fd0".
It's possible to run it on a real hardware. "run_empty.sh" automates the
testing process by creating a minimal initrd image with "init" and "test"
binaries and running the kernel in QEMU.
The tests cover these regressions:
- commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
- commit ff06db1efb2a ("floppy: fix open(O_ACCMODE) for ioctl-only open")
Signed-off-by: Denis Efremov <[email protected]>
---
MAINTAINERS | 1 +
tools/testing/selftests/floppy/.gitignore | 5 ++
tools/testing/selftests/floppy/Makefile | 10 ++++
tools/testing/selftests/floppy/config | 1 +
tools/testing/selftests/floppy/empty.c | 58 +++++++++++++++++++++
tools/testing/selftests/floppy/init.c | 43 +++++++++++++++
tools/testing/selftests/floppy/lib.sh | 57 ++++++++++++++++++++
tools/testing/selftests/floppy/run_empty.sh | 16 ++++++
8 files changed, 191 insertions(+)
create mode 100644 tools/testing/selftests/floppy/.gitignore
create mode 100644 tools/testing/selftests/floppy/Makefile
create mode 100644 tools/testing/selftests/floppy/config
create mode 100644 tools/testing/selftests/floppy/empty.c
create mode 100644 tools/testing/selftests/floppy/init.c
create mode 100644 tools/testing/selftests/floppy/lib.sh
create mode 100755 tools/testing/selftests/floppy/run_empty.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index fd25e4ecf0b9..f2a08b793b54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7227,6 +7227,7 @@ M: Denis Efremov <[email protected]>
L: [email protected]
S: Odd Fixes
F: drivers/block/floppy.c
+F: tools/testing/selftests/floppy/
FLYSKY FSIA6B RC RECEIVER
M: Markus Koch <[email protected]>
diff --git a/tools/testing/selftests/floppy/.gitignore b/tools/testing/selftests/floppy/.gitignore
new file mode 100644
index 000000000000..a9fdf1dbaa97
--- /dev/null
+++ b/tools/testing/selftests/floppy/.gitignore
@@ -0,0 +1,5 @@
+init
+cpio_list
+initrd
+
+empty
diff --git a/tools/testing/selftests/floppy/Makefile b/tools/testing/selftests/floppy/Makefile
new file mode 100644
index 000000000000..83e18cd63210
--- /dev/null
+++ b/tools/testing/selftests/floppy/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS := -static -I../../../../usr/include
+
+TEST_PROGS := run_empty.sh
+TEST_GEN_FILES := init empty
+TEST_FILES := lib.sh
+
+include ../lib.mk
+
+EXTRA_CLEAN := initrd cpio_list
diff --git a/tools/testing/selftests/floppy/config b/tools/testing/selftests/floppy/config
new file mode 100644
index 000000000000..d0c2cd8edfb2
--- /dev/null
+++ b/tools/testing/selftests/floppy/config
@@ -0,0 +1 @@
+CONFIG_BLK_DEV_FD=y
diff --git a/tools/testing/selftests/floppy/empty.c b/tools/testing/selftests/floppy/empty.c
new file mode 100644
index 000000000000..d7e2e6a74132
--- /dev/null
+++ b/tools/testing/selftests/floppy/empty.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <fcntl.h>
+#include <unistd.h>
+#include "../kselftest_harness.h"
+
+FIXTURE(floppy) {
+ const char *dev;
+ int fd;
+};
+
+FIXTURE_VARIANT(floppy) {
+ int flags;
+};
+
+/*
+ * See ff06db1efb2a ("floppy: fix open(O_ACCMODE) for ioctl-only open")
+ * fdutils use O_ACCMODE for probing and ioctl-only open
+ */
+FIXTURE_VARIANT_ADD(floppy, ACCMODE) {
+ .flags = O_ACCMODE,
+};
+
+FIXTURE_VARIANT_ADD(floppy, NACCMODE) {
+ .flags = O_ACCMODE|O_NDELAY,
+};
+
+FIXTURE_VARIANT_ADD(floppy, NRD) {
+ .flags = O_RDONLY|O_NDELAY,
+};
+
+FIXTURE_VARIANT_ADD(floppy, NWR) {
+ .flags = O_WRONLY|O_NDELAY,
+};
+
+FIXTURE_VARIANT_ADD(floppy, NRDWR) {
+ .flags = O_RDWR|O_NDELAY,
+};
+
+FIXTURE_SETUP(floppy)
+{
+ self->dev = "/dev/fd0";
+ if (access(self->dev, F_OK))
+ ksft_exit_skip("No floppy device found\n");
+};
+
+FIXTURE_TEARDOWN(floppy)
+{
+ ASSERT_EQ(close(self->fd), 0);
+}
+
+TEST_F(floppy, open)
+{
+ self->fd = open(self->dev, variant->flags);
+ ASSERT_GT(self->fd, 0);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/floppy/init.c b/tools/testing/selftests/floppy/init.c
new file mode 100644
index 000000000000..d4df4fcfd21f
--- /dev/null
+++ b/tools/testing/selftests/floppy/init.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <unistd.h>
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <sys/reboot.h>
+
+__attribute__((noreturn)) static void poweroff(void)
+{
+ fflush(stdout);
+ fflush(stderr);
+ reboot(RB_POWER_OFF);
+ sleep(10);
+ fprintf(stderr, "\nFailed to power off\n");
+ exit(1);
+}
+
+static void panic(const char *what)
+{
+ fprintf(stderr, "\nPANIC %s: %s\n", what, strerror(errno));
+ poweroff();
+}
+
+int main(int argc, char *argv[])
+{
+ pid_t pid;
+
+ pid = fork();
+ if (pid == -1)
+ panic("fork");
+ else if (pid == 0) {
+ execl("/test", "test", NULL);
+ panic("exec");
+ }
+ if (waitpid(pid, NULL, 0) < 0)
+ panic("waitpid");
+
+ poweroff();
+ return 1;
+}
diff --git a/tools/testing/selftests/floppy/lib.sh b/tools/testing/selftests/floppy/lib.sh
new file mode 100644
index 000000000000..22b711ec32ee
--- /dev/null
+++ b/tools/testing/selftests/floppy/lib.sh
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TOP_DIR="$(realpath "$(dirname "$0")"/../../../../)"
+
+QEMU=qemu-system-x86_64
+KERNEL="$TOP_DIR"/arch/x86/boot/bzImage
+INITRD=initrd
+
+gen_cpio_list() {
+ echo "file /init init 755 0 0" > cpio_list
+ echo "file /test $1 755 0 0" >> cpio_list
+ echo "dir /dev 755 0 0" >> cpio_list
+ echo "nod /dev/console 644 0 0 c 5 1" >> cpio_list
+ echo "nod /dev/fd0 666 0 0 b 2 0" >> cpio_list
+ echo "dir /mnt 755 0 0" >> cpio_list
+}
+
+gen_initrd() {
+ "$TOP_DIR"/usr/gen_initramfs.sh -o initrd ./cpio_list
+}
+
+get_selftest_log() {
+ perl -ne '$begin = $_ =~ /^TAP version/ unless $begin;
+ if ($begin && !$end) {
+ $_ =~ s/^\s+|\s+$|\[.+//g;
+ print($_ . "\n") if $_;
+ }
+ $end = $_ =~ /^# Totals:/ unless $end;'
+}
+
+run_qemu() {
+ $QEMU -enable-kvm -nodefaults -nographic \
+ -kernel "$KERNEL" -initrd "$INITRD" \
+ -append "console=ttyS0 earlyprintk=serial" \
+ -serial stdio -monitor none -no-reboot "$@"
+}
+
+run_qemu_debug() {
+ run_qemu "$@"
+}
+
+run_qemu_nodebug() {
+ run_qemu "$@" | get_selftest_log
+}
+
+detect_debug() {
+ if [ "x$1" = "x" ]; then
+ run=run_qemu_nodebug
+ else
+ run=run_qemu_debug
+ fi
+}
+
+run_qemu_empty() {
+ detect_debug "$1"
+ $run -drive index=0,if=floppy
+}
diff --git a/tools/testing/selftests/floppy/run_empty.sh b/tools/testing/selftests/floppy/run_empty.sh
new file mode 100755
index 000000000000..f018107dc6a5
--- /dev/null
+++ b/tools/testing/selftests/floppy/run_empty.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+source "$(dirname $0)"/lib.sh
+
+while getopts d flag; do
+ case "${flag}" in
+ d) debug=1;;
+ esac
+done
+
+gen_cpio_list empty
+gen_initrd empty
+run_qemu_empty $debug
--
2.31.1
Properly handle commits like:
commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
Cc: Joe Perches <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
scripts/checkpatch.pl | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 461d4221e4a4..cf31e8c994d3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3200,20 +3200,20 @@ sub process {
$long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
$space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
$case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
- if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
+ if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)"\)/i) {
$orig_desc = $1;
$hasparens = 1;
} elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
defined $rawlines[$linenr] &&
- $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
+ $rawlines[$linenr] =~ /^\s*\("(.+)"\)/) {
$orig_desc = $1;
$hasparens = 1;
- } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
+ } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\(".+$/i &&
defined $rawlines[$linenr] &&
- $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
- $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
+ $rawlines[$linenr] =~ /^\s*.+"\)/) {
+ $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)$/i;
$orig_desc = $1;
- $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
+ $rawlines[$linenr] =~ /^\s*(.+)"\)/;
$orig_desc .= " " . $1;
$hasparens = 1;
}
--
2.31.1
Add basic tests for a floppy with writable disk. "rdwr" test
works under following assumptions:
- writable disk in "/dev/fd0"
To simulate the conditions and automate the testing process there is
"run_rdwr.sh".
Signed-off-by: Denis Efremov <[email protected]>
---
tools/testing/selftests/floppy/.gitignore | 1 +
tools/testing/selftests/floppy/Makefile | 4 +-
tools/testing/selftests/floppy/lib.sh | 4 ++
tools/testing/selftests/floppy/rdwr.c | 67 ++++++++++++++++++++++
tools/testing/selftests/floppy/run_rdwr.sh | 22 +++++++
5 files changed, 96 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/floppy/rdwr.c
create mode 100755 tools/testing/selftests/floppy/run_rdwr.sh
diff --git a/tools/testing/selftests/floppy/.gitignore b/tools/testing/selftests/floppy/.gitignore
index 7642dc0ef281..f53e70197edd 100644
--- a/tools/testing/selftests/floppy/.gitignore
+++ b/tools/testing/selftests/floppy/.gitignore
@@ -5,3 +5,4 @@ testdir/
empty
rdonly
+rdwr
diff --git a/tools/testing/selftests/floppy/Makefile b/tools/testing/selftests/floppy/Makefile
index ed8fdeb79aea..c5d010dd4445 100644
--- a/tools/testing/selftests/floppy/Makefile
+++ b/tools/testing/selftests/floppy/Makefile
@@ -1,8 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS := -static -I../../../../usr/include
-TEST_PROGS := run_empty.sh run_rdonly.sh
-TEST_GEN_FILES := init empty rdonly
+TEST_PROGS := run_empty.sh run_rdonly.sh run_rdwr.sh
+TEST_GEN_FILES := init empty rdonly rdwr
TEST_FILES := lib.sh
include ../lib.mk
diff --git a/tools/testing/selftests/floppy/lib.sh b/tools/testing/selftests/floppy/lib.sh
index 9988be187bc9..0eab702b355a 100644
--- a/tools/testing/selftests/floppy/lib.sh
+++ b/tools/testing/selftests/floppy/lib.sh
@@ -61,3 +61,7 @@ run_qemu_rdonly_fat() {
$run -drive file=fat:floppy:"$1",index=0,if=floppy,readonly
}
+run_qemu_rdwr_img() {
+ detect_debug "$2"
+ $run -drive file="$1",index=0,if=floppy,format=raw
+}
diff --git a/tools/testing/selftests/floppy/rdwr.c b/tools/testing/selftests/floppy/rdwr.c
new file mode 100644
index 000000000000..44ad18701530
--- /dev/null
+++ b/tools/testing/selftests/floppy/rdwr.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/mount.h>
+#include <errno.h>
+#include <linux/fd.h>
+#include "../kselftest_harness.h"
+
+FIXTURE(floppy) {
+ const char *dev;
+};
+
+FIXTURE_SETUP(floppy)
+{
+ int fd;
+ struct floppy_drive_params params;
+
+ self->dev = "/dev/fd0";
+ if (access(self->dev, F_OK))
+ ksft_exit_skip("No floppy device found\n");
+ if (access(self->dev, R_OK))
+ ksft_exit_skip("Floppy is not read accessible\n");
+ if (access(self->dev, W_OK))
+ ksft_exit_skip("Floppy is not write accessible\n");
+
+ fd = open("/dev/fd0", O_ACCMODE|O_NDELAY);
+ EXPECT_EQ(0, ioctl(fd, FDGETDRVPRM, ¶ms));
+ params.flags |= FD_DEBUG;
+ EXPECT_EQ(0, ioctl(fd, FDSETDRVPRM, ¶ms));
+ close(fd);
+}
+
+FIXTURE_TEARDOWN(floppy)
+{
+}
+
+TEST_F(floppy, write)
+{
+#define TEST_DATA "TEST_WRITE"
+ int fd;
+ char test[] = TEST_DATA;
+
+ fd = open(self->dev, O_RDWR);
+ ASSERT_GT(fd, 0);
+
+ ASSERT_EQ(sizeof(test), write(fd, test, sizeof(test)));
+ ASSERT_EQ(sizeof(test), read(fd, test, sizeof(test)));
+ ASSERT_NE(0, strncmp(TEST_DATA, test, sizeof(TEST_DATA)));
+
+ ASSERT_EQ(close(fd), 0);
+#undef TEST_DATA
+}
+
+TEST_F(floppy, ioctl_disk_writable)
+{
+ int fd;
+ struct floppy_drive_struct drive;
+
+ fd = open(self->dev, O_RDONLY|O_NDELAY);
+ ASSERT_GT(fd, 0);
+ ASSERT_EQ(0, ioctl(fd, FDGETDRVSTAT, &drive));
+ ASSERT_TRUE(drive.flags & FD_DISK_WRITABLE);
+ ASSERT_EQ(close(fd), 0);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/floppy/run_rdwr.sh b/tools/testing/selftests/floppy/run_rdwr.sh
new file mode 100755
index 000000000000..0ebe8bd6bc69
--- /dev/null
+++ b/tools/testing/selftests/floppy/run_rdwr.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+source "$(dirname $0)"/lib.sh
+
+while getopts d flag; do
+ case "${flag}" in
+ d) debug=1;;
+ esac
+done
+
+if [ -z $debug ]; then
+ trap "rm -rf testdir" EXIT
+fi
+mkdir -p testdir
+head -c 1474560 /dev/zero > testdir/floppy.raw
+
+gen_cpio_list rdwr
+gen_initrd rdwr
+run_qemu_rdwr_img testdir/floppy.raw $debug
--
2.31.1
Add basic tests for a floppy with a readonly disk. "rdonly" test
works under following assumptions:
- there is a readonly disk in "/dev/fd0"
- the disk is VFAT formatted
- there is "test" file with "TEST" in it on the disk
"run_rdonly.sh" script simulates the conditions and automates the
testing process.
The tests cover the regression:
- commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
Signed-off-by: Denis Efremov <[email protected]>
---
tools/testing/selftests/floppy/.gitignore | 2 +
tools/testing/selftests/floppy/Makefile | 6 +-
tools/testing/selftests/floppy/lib.sh | 6 ++
tools/testing/selftests/floppy/rdonly.c | 99 ++++++++++++++++++++
tools/testing/selftests/floppy/run_rdonly.sh | 22 +++++
5 files changed, 132 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/floppy/rdonly.c
create mode 100755 tools/testing/selftests/floppy/run_rdonly.sh
diff --git a/tools/testing/selftests/floppy/.gitignore b/tools/testing/selftests/floppy/.gitignore
index a9fdf1dbaa97..7642dc0ef281 100644
--- a/tools/testing/selftests/floppy/.gitignore
+++ b/tools/testing/selftests/floppy/.gitignore
@@ -1,5 +1,7 @@
init
cpio_list
initrd
+testdir/
empty
+rdonly
diff --git a/tools/testing/selftests/floppy/Makefile b/tools/testing/selftests/floppy/Makefile
index 83e18cd63210..ed8fdeb79aea 100644
--- a/tools/testing/selftests/floppy/Makefile
+++ b/tools/testing/selftests/floppy/Makefile
@@ -1,10 +1,10 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS := -static -I../../../../usr/include
-TEST_PROGS := run_empty.sh
-TEST_GEN_FILES := init empty
+TEST_PROGS := run_empty.sh run_rdonly.sh
+TEST_GEN_FILES := init empty rdonly
TEST_FILES := lib.sh
include ../lib.mk
-EXTRA_CLEAN := initrd cpio_list
+EXTRA_CLEAN := initrd cpio_list testdir
diff --git a/tools/testing/selftests/floppy/lib.sh b/tools/testing/selftests/floppy/lib.sh
index 22b711ec32ee..9988be187bc9 100644
--- a/tools/testing/selftests/floppy/lib.sh
+++ b/tools/testing/selftests/floppy/lib.sh
@@ -55,3 +55,9 @@ run_qemu_empty() {
detect_debug "$1"
$run -drive index=0,if=floppy
}
+
+run_qemu_rdonly_fat() {
+ detect_debug "$2"
+ $run -drive file=fat:floppy:"$1",index=0,if=floppy,readonly
+}
+
diff --git a/tools/testing/selftests/floppy/rdonly.c b/tools/testing/selftests/floppy/rdonly.c
new file mode 100644
index 000000000000..fa60d069540b
--- /dev/null
+++ b/tools/testing/selftests/floppy/rdonly.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/mount.h>
+#include <sys/ioctl.h>
+#include <linux/fd.h>
+#include "../kselftest_harness.h"
+
+FIXTURE(floppy) {
+ const char *dev;
+};
+
+FIXTURE_SETUP(floppy)
+{
+ int fd;
+ struct floppy_drive_params params;
+
+ self->dev = "/dev/fd0";
+ if (access(self->dev, F_OK))
+ ksft_exit_skip("No floppy device found\n");
+ if (access(self->dev, R_OK))
+ ksft_exit_skip("Floppy is not read accessible\n");
+
+ fd = open(self->dev, O_ACCMODE|O_NDELAY);
+ EXPECT_EQ(0, ioctl(fd, FDGETDRVPRM, ¶ms));
+ params.flags |= FTD_MSG|FD_DEBUG;
+ EXPECT_EQ(0, ioctl(fd, FDSETDRVPRM, ¶ms));
+ close(fd);
+}
+
+FIXTURE_TEARDOWN(floppy)
+{
+}
+
+TEST_F(floppy, read)
+{
+ int fd, test;
+
+ fd = open(self->dev, O_RDONLY);
+ ASSERT_GT(fd, 0);
+ ASSERT_EQ(read(fd, &test, sizeof(test)), sizeof(test));
+ ASSERT_EQ(close(fd), 0);
+}
+
+TEST_F(floppy, open_write_fail)
+{
+ ASSERT_LT(open(self->dev, O_WRONLY), 0);
+}
+
+TEST_F(floppy, open_rdwr_fail)
+{
+ ASSERT_LT(open(self->dev, O_RDWR), 0);
+}
+
+TEST_F(floppy, ioctl_disk_writable)
+{
+ int fd;
+ struct floppy_drive_struct drive;
+
+ fd = open(self->dev, O_RDONLY|O_NDELAY);
+ ASSERT_GT(fd, 0);
+ ASSERT_EQ(0, ioctl(fd, FDGETDRVSTAT, &drive));
+ ASSERT_FALSE(drive.flags & FD_DISK_WRITABLE);
+ ASSERT_EQ(close(fd), 0);
+}
+
+TEST_F(floppy, mount)
+{
+ int fd;
+ char test[5] = {};
+
+ mount(self->dev, "/mnt", "vfat", MS_RDONLY, NULL);
+ ASSERT_EQ(0, errno);
+
+ fd = open("/mnt/test", O_RDONLY);
+ read(fd, &test, sizeof(test));
+ ASSERT_EQ(0, strncmp(test, "TEST", 4));
+}
+
+TEST_F(floppy, open_ndelay_write_fail)
+{
+#define TEST_DATA "TEST_FAIL_WRITE"
+ int fd;
+ char test[] = TEST_DATA;
+
+ fd = open(self->dev, O_RDWR|O_NDELAY);
+ ASSERT_GT(fd, 0);
+
+ write(fd, test, sizeof(test));
+ read(fd, test, sizeof(test));
+ ASSERT_NE(0, strncmp(TEST_DATA, test, sizeof(TEST_DATA)));
+
+ ASSERT_EQ(close(fd), 0);
+#undef TEST_DATA
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/floppy/run_rdonly.sh b/tools/testing/selftests/floppy/run_rdonly.sh
new file mode 100755
index 000000000000..a358a1b6e58d
--- /dev/null
+++ b/tools/testing/selftests/floppy/run_rdonly.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+source "$(dirname $0)"/lib.sh
+
+while getopts d flag; do
+ case "${flag}" in
+ d) debug=1;;
+ esac
+done
+
+if [ -z $debug ]; then
+ trap "rm -rf testdir" EXIT
+fi
+mkdir -p testdir
+echo -n TEST > testdir/test
+
+gen_cpio_list rdonly
+gen_initrd rdonly
+run_qemu_rdonly_fat testdir $debug
--
2.31.1
On Wed, 2021-08-18 at 18:46 +0300, Denis Efremov wrote:
> Properly handle commits like:
> commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
Try this one:
https://lore.kernel.org/lkml/[email protected]/
>
> Cc: Joe Perches <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> ?scripts/checkpatch.pl | 12 ++++++------
> ?1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 461d4221e4a4..cf31e8c994d3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3200,20 +3200,20 @@ sub process {
> ? $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
> ? $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
> ? $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
> - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
> + if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)"\)/i) {
> ? $orig_desc = $1;
> ? $hasparens = 1;
> ? } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
> ? defined $rawlines[$linenr] &&
> - $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
> + $rawlines[$linenr] =~ /^\s*\("(.+)"\)/) {
> ? $orig_desc = $1;
> ? $hasparens = 1;
> - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
> + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\(".+$/i &&
> ? defined $rawlines[$linenr] &&
> - $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
> - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
> + $rawlines[$linenr] =~ /^\s*.+"\)/) {
> + $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)$/i;
> ? $orig_desc = $1;
> - $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
> + $rawlines[$linenr] =~ /^\s*(.+)"\)/;
> ? $orig_desc .= " " . $1;
> ? $hasparens = 1;
> ? }
On 8/18/21 7:00 PM, Joe Perches wrote:
> On Wed, 2021-08-18 at 18:46 +0300, Denis Efremov wrote:
>> Properly handle commits like:
>> commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
>
> Try this one:
>
> https://lore.kernel.org/lkml/[email protected]/
>
It works but why not to use .+? then?
I'm not sure that non-greedy patterns will properly handle commits like:
$ git log --oneline | fgrep '")'
e.g.
commit ece2619fe8ed ("extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call")
>>
>> Cc: Joe Perches <[email protected]>
>> Signed-off-by: Denis Efremov <[email protected]>
>> ---
>> scripts/checkpatch.pl | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 461d4221e4a4..cf31e8c994d3 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -3200,20 +3200,20 @@ sub process {
>> $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
>> $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
>> $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
>> - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
>> + if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)"\)/i) {
>> $orig_desc = $1;
>> $hasparens = 1;
>> } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
>> defined $rawlines[$linenr] &&
>> - $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
>> + $rawlines[$linenr] =~ /^\s*\("(.+)"\)/) {
>> $orig_desc = $1;
>> $hasparens = 1;
>> - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
>> + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\(".+$/i &&
>> defined $rawlines[$linenr] &&
>> - $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
>> - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
>> + $rawlines[$linenr] =~ /^\s*.+"\)/) {
>> + $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)$/i;
>> $orig_desc = $1;
>> - $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
>> + $rawlines[$linenr] =~ /^\s*(.+)"\)/;
>> $orig_desc .= " " . $1;
>> $hasparens = 1;
>> }
>
On Wed, 2021-08-18 at 19:21 +0300, Denis Efremov wrote:
>
> On 8/18/21 7:00 PM, Joe Perches wrote:
> > On Wed, 2021-08-18 at 18:46 +0300, Denis Efremov wrote:
> > > Properly handle commits like:
> > > commit f2791e7eadf4 ("Revert "floppy: refactor open() flags handling"")
> >
> > Try this one:
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
>
> It works but why not to use .+? then?
> I'm not sure that non-greedy patterns will properly handle commits like:
> $ git log --oneline | fgrep '")'
>
> e.g.
> commit ece2619fe8ed ("extcon: arizona: Fix flags parameter to the gpiod_get("wlf,micd-pol") call")
The only way to handle that is to use the $balanced_parens test but
it wouldn't work on Andrew's perl version 5.8
Andrew? Do you still use perl 5.8? It's almost 20 years old now.
Does anyone still use perl versions 5.8 or lower?
From checkpatch:
# Using $balanced_parens, $LvalOrFunc, or $FuncArg
# requires at least perl version v5.10.0
# Any use must be runtime checked with $^V
our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/;
our $LvalOrFunc = qr{((?:[\&\*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*};
our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)};
So maybe:
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > > @@ -3200,20 +3200,20 @@ sub process {
> > > ? $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
> > > ? $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
> > > ? $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
> > > - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
> > > + if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)"\)/i) {
So that could be:
if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i
> > > ? $orig_desc = $1;
> > > ? $hasparens = 1;
> > > ? } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
> > > ? defined $rawlines[$linenr] &&
> > > - $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
> > > + $rawlines[$linenr] =~ /^\s*\("(.+)"\)/) {
and
? } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
? defined $rawlines[$linenr] &&
"$line $rawlines[$linenr]" =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i
> > > ? $orig_desc = $1;
> > > ? $hasparens = 1;
> > > - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
> > > + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\(".+$/i &&
> > > ? defined $rawlines[$linenr] &&
> > > - $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
> > > - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
> > > + $rawlines[$linenr] =~ /^\s*.+"\)/) {
> > > + $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(.+)$/i;
etc...
> > > ? $orig_desc = $1;
> > > - $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
> > > + $rawlines[$linenr] =~ /^\s*(.+)"\)/;
> > > ? $orig_desc .= " " . $1;
> > > ? $hasparens = 1;
> > > ? }
> >
On Wed, 18 Aug 2021 13:21:16 -0700 Joe Perches <[email protected]> wrote:
> Andrew? Do you still use perl 5.8? It's almost 20 years old now.
5.26 is the oldest I appear to be using.
Hey Denis:
Try this one please and let me know what you think...
---
scripts/checkpatch.pl | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 161ce7fe5d1e5..4e2e79eff9b8c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3196,26 +3196,21 @@ sub process {
$orig_commit = lc($1);
}
- $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i);
- $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
- $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
- $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
- if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
- $orig_desc = $1;
- $hasparens = 1;
- } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
- defined $rawlines[$linenr] &&
- $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
- $orig_desc = $1;
- $hasparens = 1;
- } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
- defined $rawlines[$linenr] &&
- $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
- $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
+ my $input = $line;
+ for (my $n = 0; $n < 2; $n++) {
+ $input .= " $rawlines[$linenr + $n]" if ($#lines >= $linenr + $n);
+ }
+
+ $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i);
+ $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i);
+ $space = 0 if ($input =~ /\bcommit [0-9a-f]/i);
+ $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
+ if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i) {
$orig_desc = $1;
- $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
- $orig_desc .= " " . $1;
$hasparens = 1;
+ # Always strip leading/trailing parens then double quotes if existing
+ $orig_desc = substr($orig_desc, 1, -1);
+ $orig_desc = substr($orig_desc, 1, -1) if ($orig_desc =~ /^".*"$/);
}
($id, $description) = git_commit_info($orig_commit,
On Thu, Aug 19, 2021 at 12:47 AM Denis Efremov <[email protected]> wrote:
>
> Use absolute path to call gen_init_cpio. This allows one
> to use gen_initramfs.sh from any directory.
I do not mind this, but $(dirname "$0")
is not necessarily an absolute path, is it?
I added test code:
echo dirname is $(dirname $0)
in this script, and I saw
dirname is usr
>
> Cc: Masahiro Yamada <[email protected]>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> usr/gen_initramfs.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
> index 63476bb70b41..2e4a86181c79 100755
> --- a/usr/gen_initramfs.sh
> +++ b/usr/gen_initramfs.sh
> @@ -244,4 +244,4 @@ if test -n "$KBUILD_BUILD_TIMESTAMP"; then
> timestamp="-t $timestamp"
> fi
> fi
> -usr/gen_init_cpio $timestamp $cpio_list > $output
> +"$(dirname "$0")"/gen_init_cpio $timestamp $cpio_list > $output
> --
> 2.31.1
>
--
Best Regards
Masahiro Yamada
Hi,
On 8/19/21 12:22 AM, Joe Perches wrote:
> Hey Denis:
>
> Try this one please and let me know what you think...
Looks good to me. Couple of nitpicks below
>
> ---
> scripts/checkpatch.pl | 31 +++++++++++++------------------
> 1 file changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 161ce7fe5d1e5..4e2e79eff9b8c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3196,26 +3196,21 @@ sub process {
> $orig_commit = lc($1);
> }
>
> - $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i);
> - $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
> - $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
> - $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
> - if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
> - $orig_desc = $1;
> - $hasparens = 1;
> - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
> - defined $rawlines[$linenr] &&
> - $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
> - $orig_desc = $1;
> - $hasparens = 1;
> - } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
> - defined $rawlines[$linenr] &&
> - $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
> - $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
> + my $input = $line;
> + for (my $n = 0; $n < 2; $n++) {
> + $input .= " $rawlines[$linenr + $n]" if ($#lines >= $linenr + $n);
> + }
> +
> + $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i);
> + $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i);
> + $space = 0 if ($input =~ /\bcommit [0-9a-f]/i);
> + $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
> + if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i) {
> $orig_desc = $1;
> - $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
> - $orig_desc .= " " . $1;
> $hasparens = 1;
> + # Always strip leading/trailing parens then double quotes if existing
> + $orig_desc = substr($orig_desc, 1, -1);
> + $orig_desc = substr($orig_desc, 1, -1) if ($orig_desc =~ /^".*"$/);
Why do you want to add "if ($orig_desc =~ /^".*"$/);" here? and not just substr($orig_desc, 2, -2);?
> }
>
> ($id, $description) = git_commit_info($orig_commit,
>
In your previous patch with '.*?' you added a branch to allow also newlines between commit and shas:
```
commit
c3f157259438 (Revert "floppy: reintroduce O_NDELAY fix")
```
Maybe something like this will work (adding a last word from a prevline if line doesn't start from
commit)
+ my $input = $line;
if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
$init_char = $1;
$orig_commit = lc($2);
} elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) {
$orig_commit = lc($1);
+ $prevline =~ /(\w+)$/;
+ $line = $1 . " " . $prevline;
}
- my $input = $line;
for (my $n = 0; $n < 2; $n++) {
$input .= " $rawlines[$linenr + $n]" if ($#lines >= $linenr + $n);
}
Thanks,
Denis
On 8/19/21 3:24 AM, Masahiro Yamada wrote:
> On Thu, Aug 19, 2021 at 12:47 AM Denis Efremov <[email protected]> wrote:
>>
>> Use absolute path to call gen_init_cpio. This allows one
>> to use gen_initramfs.sh from any directory.
>
> I do not mind this, but $(dirname "$0")
> is not necessarily an absolute path, is it?
>
>
> I added test code:
>
> echo dirname is $(dirname $0)
>
> in this script, and I saw
>
> dirname is usr
Oh, sorry, commit message is wrong. Would that be ok for you if I will change
it in v2 to something like:
Prepend gen_init_cpio call with the same path as gen_initramfs.sh called. This
allows one to use gen_initramfs.sh from any directory, not only from the
kernel's topdir.
>
>
>
>
>>
>> Cc: Masahiro Yamada <[email protected]>
>> Signed-off-by: Denis Efremov <[email protected]>
>> ---
>> usr/gen_initramfs.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
>> index 63476bb70b41..2e4a86181c79 100755
>> --- a/usr/gen_initramfs.sh
>> +++ b/usr/gen_initramfs.sh
>> @@ -244,4 +244,4 @@ if test -n "$KBUILD_BUILD_TIMESTAMP"; then
>> timestamp="-t $timestamp"
>> fi
>> fi
>> -usr/gen_init_cpio $timestamp $cpio_list > $output
>> +"$(dirname "$0")"/gen_init_cpio $timestamp $cpio_list > $output
>> --
>> 2.31.1
>>
>
>
> --
> Best Regards
> Masahiro Yamada
>
On Thu, 2021-08-19 at 22:52 +0300, Denis Efremov wrote:
> Hi,
>
> On 8/19/21 12:22 AM, Joe Perches wrote:
> > Hey Denis:
> >
> > Try this one please and let me know what you think...
>
> Looks good to me. Couple of nitpicks below
yeah, thanks.
How about this one:
---
scripts/checkpatch.pl | 72 ++++++++++++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 161ce7fe5d1e5..4988515a0dfb3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1181,7 +1181,8 @@ sub git_commit_info {
# git log --format='%H %s' -1 $line |
# echo "commit $(cut -c 1-12,41-)"
# done
- } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree\./) {
+ } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree\./ ||
+ $lines[0] =~ /^fatal: bad object $commit/) {
$id = undef;
} else {
$id = substr($lines[0], 0, 12);
@@ -2587,6 +2588,8 @@ sub process {
my $reported_maintainer_file = 0;
my $non_utf8_charset = 0;
+ my $last_git_commit_id_linenr = -1;
+
my $last_blank_line = 0;
my $last_coalesced_string_linenr = -1;
@@ -3173,7 +3176,8 @@ sub process {
if ($in_commit_log && !$commit_log_possible_stack_dump &&
$line !~ /^\s*(?:Link|Patchwork|http|https|BugLink|base-commit):/i &&
$line !~ /^This reverts commit [0-9a-f]{7,40}/ &&
- ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i ||
+ (($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i ||
+ ($line =~ /\bcommit\s*$/i && defined($rawlines[$linenr]) && $rawlines[$linenr] =~ /^\s*[0-9a-f]{5,}\b/i)) ||
($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i &&
$line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i &&
$line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) {
@@ -3183,49 +3187,59 @@ sub process {
my $long = 0;
my $case = 1;
my $space = 1;
- my $hasdesc = 0;
my $hasparens = 0;
my $id = '0123456789ab';
my $orig_desc = "commit description";
my $description = "";
+ my $herectx = $herecurr;
+ my $has_parens = 0;
+
+ my $input = $line;
+ if ($line =~ /(?:\bcommit\s+[0-9a-f]{5,}|\bcommit\s*$)/i) {
+ for (my $n = 0; $n < 2; $n++) {
+ if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s*$balanced_parens/i) {
+ $has_parens = 1;
+ last;
+ }
+ last if ($#lines < $linenr + $n);
+ $input .= " " . trim($rawlines[$linenr + $n]);
+ $herectx .= "$rawlines[$linenr + $n]\n";
+ }
+ $herectx = $herecurr if (!$has_parens);
+ }
- if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
+ if ($input =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) {
$init_char = $1;
$orig_commit = lc($2);
- } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) {
+ $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i);
+ $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i);
+ $space = 0 if ($input =~ /\bcommit [0-9a-f]/i);
+ $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
+
+ if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s+($balanced_parens)/i) {
+ $orig_desc = $1;
+ # Always strip leading/trailing parens then double quotes if existing
+ $orig_desc = substr($orig_desc, 1, -1);
+ if ($orig_desc =~ /^".*"$/) {
+ $orig_desc = substr($orig_desc, 1, -1);
+ $hasparens = 1;
+ }
+ }
+ } elsif ($input =~ /\b([0-9a-f]{12,40})\b/i) {
$orig_commit = lc($1);
}
- $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i);
- $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i);
- $space = 0 if ($line =~ /\bcommit [0-9a-f]/i);
- $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/);
- if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) {
- $orig_desc = $1;
- $hasparens = 1;
- } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i &&
- defined $rawlines[$linenr] &&
- $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) {
- $orig_desc = $1;
- $hasparens = 1;
- } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i &&
- defined $rawlines[$linenr] &&
- $rawlines[$linenr] =~ /^\s*[^"]+"\)/) {
- $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i;
- $orig_desc = $1;
- $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
- $orig_desc .= " " . $1;
- $hasparens = 1;
- }
-
($id, $description) = git_commit_info($orig_commit,
$id, $orig_desc);
if (defined($id) &&
- ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens)) {
+ ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens) &&
+ $last_git_commit_id_linenr != $linenr - 1) {
ERROR("GIT_COMMIT_ID",
- "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herecurr);
+ "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herectx);
}
+ #don't report the next line if this line ends in commit and the sha1 hash is the next line
+ $last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
}
# Check for added, moved or deleted files
On Thu, 2021-08-19 at 22:52 +0300, Denis Efremov wrote:
> Hi,
>
> Why do you want to add "if ($orig_desc =~ /^".*"$/);" here? and not just substr($orig_desc, 2, -2);?
Because commit descriptions sometimes to not have quotes like
commit <deadbeef> (Multiple word description)
btw:
I tested the last proposal with this script:
$ git log --grep="commit [0-9a-f]" -i --format=%h -1000 | \
while read commit ; do \
echo $commit; \
./scripts/checkpatch.pl --git --no-summary --quiet --types=GIT_COMMIT_ID $commit ; \
done
and there are still a fair number of ERRORs.
And I'm not sure if this particular ERROR is that useful overall.
On Fri, Aug 20, 2021 at 5:51 AM Denis Efremov <[email protected]> wrote:
>
>
>
> On 8/19/21 3:24 AM, Masahiro Yamada wrote:
> > On Thu, Aug 19, 2021 at 12:47 AM Denis Efremov <[email protected]> wrote:
> >>
> >> Use absolute path to call gen_init_cpio. This allows one
> >> to use gen_initramfs.sh from any directory.
> >
> > I do not mind this, but $(dirname "$0")
> > is not necessarily an absolute path, is it?
> >
> >
> > I added test code:
> >
> > echo dirname is $(dirname $0)
> >
> > in this script, and I saw
> >
> > dirname is usr
>
> Oh, sorry, commit message is wrong. Would that be ok for you if I will change
> it in v2 to something like:
>
> Prepend gen_init_cpio call with the same path as gen_initramfs.sh called. This
> allows one to use gen_initramfs.sh from any directory, not only from the
> kernel's topdir.
I am fine with it.
This patch is prefixed with 2/5, so I assume
you expect another person to pick up
the entire series.
With the commit message updated,
Reviewed-by: Masahiro Yamada <[email protected]>
> >
> >
> >
> >
> >>
> >> Cc: Masahiro Yamada <[email protected]>
> >> Signed-off-by: Denis Efremov <[email protected]>
> >> ---
> >> usr/gen_initramfs.sh | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/usr/gen_initramfs.sh b/usr/gen_initramfs.sh
> >> index 63476bb70b41..2e4a86181c79 100755
> >> --- a/usr/gen_initramfs.sh
> >> +++ b/usr/gen_initramfs.sh
> >> @@ -244,4 +244,4 @@ if test -n "$KBUILD_BUILD_TIMESTAMP"; then
> >> timestamp="-t $timestamp"
> >> fi
> >> fi
> >> -usr/gen_init_cpio $timestamp $cpio_list > $output
> >> +"$(dirname "$0")"/gen_init_cpio $timestamp $cpio_list > $output
> >> --
> >> 2.31.1
> >>
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
--
Best Regards
Masahiro Yamada
On 8/20/21 1:17 AM, Joe Perches wrote:
>
> And I'm not sure if this particular ERROR is that useful overall.
I find it useful to check commit-id and that it matches a title.
It's easy to make a typo in commit-id and get an invalid one.
Denis
On Sat, 2021-08-21 at 09:47 +0300, Denis Efremov wrote:
>
> On 8/20/21 1:17 AM, Joe Perches wrote:
> >
> > And I'm not sure if this particular ERROR is that useful overall.
>
> I find it useful to check commit-id and that it matches a title.
> It's easy to make a typo in commit-id and get an invalid one.
That's true, but I meant requiring the sha1 hash to contain both
the word "commit" and use ("title").
Looking at checkpatch's errors produced by this GIT_COMMIT_ID
test makes the required form seem a bit too inflexible to me.
For instance: a sha1 hash may be repeated in a commit message where
the first instance has the correct form but the second use is just
the hash and the warning is still produced.
On 8/21/21 10:12 AM, Joe Perches wrote:
> On Sat, 2021-08-21 at 09:47 +0300, Denis Efremov wrote:
>>
>> On 8/20/21 1:17 AM, Joe Perches wrote:
>>>
>>> And I'm not sure if this particular ERROR is that useful overall.
>>
>> I find it useful to check commit-id and that it matches a title.
>> It's easy to make a typo in commit-id and get an invalid one.
>
> That's true, but I meant requiring the sha1 hash to contain both
> the word "commit" and use ("title").
>
> Looking at checkpatch's errors produced by this GIT_COMMIT_ID
> test makes the required form seem a bit too inflexible to me.
>
> For instance: a sha1 hash may be repeated in a commit message where
> the first instance has the correct form but the second use is just
> the hash and the warning is still produced.
>
I agree with you. There is also another example with list of commits:
- commit <id-1> ("Title1")
- commit <id-2> ("Title2")
...
I see no reason in writing "commit" on each line.