Fix check-include-guards.py
Checks
Commit Message
I noticed that check-include-guards.py doesn't error in certain
situations -- but in situations where the --update flag would cause a
file to be changed.
This patch changes the script to issue an error for any discrepancy.
It also fixes the headers that weren't correct.
---
gdb/arch/loongarch-insn.h | 6 +++---
gdb/arch/loongarch-syscall.h | 6 +++---
gdb/check-include-guards.py | 19 +++++++++++++------
gdb/config/djgpp/langinfo.h | 6 +++---
gdb/config/djgpp/nl_types.h | 6 +++---
gdb/config/i386/nm-x86-gnu.h | 6 +++---
gdb/config/sparc/nm-sol2.h | 6 +++---
gdb/python/py-color.h | 6 +++---
gdbsupport/scoped_signal_handler.h | 6 +++---
9 files changed, 37 insertions(+), 30 deletions(-)
Comments
On 2025-03-09 13:09, Tom Tromey wrote:
> I noticed that check-include-guards.py doesn't error in certain
> situations -- but in situations where the --update flag would cause a
> file to be changed.
>
> This patch changes the script to issue an error for any discrepancy.
> It also fixes the headers that weren't correct.
After reading your patch, I have some possible enhancements in mind.
One of them is that if you pass multiple files to the script and there
are errors in multiple files, it will only print the first one. It
would be nice if it printed all errors. This is especially nice for CI,
where you'd want to have the list of all the problems printed.
But I think you patch is fine, since it does fix an existing problem and
doesn't make things worse otherwise.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> After reading your patch, I have some possible enhancements in mind.
Simon> One of them is that if you pass multiple files to the script and there
Simon> are errors in multiple files, it will only print the first one. It
Simon> would be nice if it printed all errors.
FWIW pre-commit seems to run it individually on each affected file.
So, "pre-commit run --all" will show all the errors in one invocation.
Tom
On 2025-03-10 11:12, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>
> Simon> After reading your patch, I have some possible enhancements in mind.
> Simon> One of them is that if you pass multiple files to the script and there
> Simon> are errors in multiple files, it will only print the first one. It
> Simon> would be nice if it printed all errors.
>
> FWIW pre-commit seems to run it individually on each affected file.
> So, "pre-commit run --all" will show all the errors in one invocation.
>
> Tom
I didn't think of running pre-commit directly in the CI, but I suppose
it would make sense, it would help enforce the rules as written out in
the repo. I'll give that a try.
Simon
@@ -18,8 +18,8 @@
/* The LoongArch opcode and mask definitions in this file are obtained from
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=opcodes/loongarch-opc.c */
-#ifndef ARCH_LOONGARCH_INSN_H
-#define ARCH_LOONGARCH_INSN_H
+#ifndef GDB_ARCH_LOONGARCH_INSN_H
+#define GDB_ARCH_LOONGARCH_INSN_H
/* loongarch fix insn opcode */
#define OP_CLO_W 0x00001000
@@ -2093,4 +2093,4 @@ is_special_insn (uint32_t insn)
return false;
}
-#endif /* ARCH_LOONGARCH_INSN_H */
+#endif /* GDB_ARCH_LOONGARCH_INSN_H */
@@ -19,8 +19,8 @@
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/unistd.h
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/loongarch/include/asm/unistd.h */
-#ifndef ARCH_LOONGARCH_SYSCALL_H
-#define ARCH_LOONGARCH_SYSCALL_H
+#ifndef GDB_ARCH_LOONGARCH_SYSCALL_H
+#define GDB_ARCH_LOONGARCH_SYSCALL_H
enum loongarch_syscall
{
@@ -345,4 +345,4 @@ enum loongarch_syscall
loongarch_sys_syscalls = 463,
};
-#endif /* ARCH_LOONGARCH_SYSCALL_H */
+#endif /* GDB_ARCH_LOONGARCH_SYSCALL_H */
@@ -99,12 +99,14 @@ def check_header(filename: str):
failure(filename, i, "no header guard")
force_rewrite = True
symbol = m.group(1)
- updated = False
+ # Either None or a tuple like (LINE, TEXT) that describes a needed
+ # update.
+ updated = None
if symbol != expected:
force_rewrite = True
if force_rewrite:
contents[i] = "#ifndef " + expected + "\n"
- updated = True
+ updated = (i, "wrong symbol in ifndef")
i += 1
if i == len(contents):
failure(filename, i, "premature EOF")
@@ -112,15 +114,20 @@ def check_header(filename: str):
failure(filename, i, "no define of header guard")
if contents[i] != "#define " + expected + "\n":
contents[i] = "#define " + expected + "\n"
- updated = True
+ if updated is None:
+ updated = (i, "wrong symbol in define")
i = len(contents) - 1
if not contents[i].startswith("#endif"):
failure(filename, i, "no trailing endif")
if contents[i] != "#endif /* " + expected + " */\n":
contents[i] = "#endif /* " + expected + " */\n"
- updated = True
- if updated and write_files:
- write_header(filename, contents)
+ if updated is None:
+ updated = (i, "wrong endif line")
+ if updated is not None:
+ if write_files:
+ write_header(filename, contents)
+ else:
+ failure(filename, *updated)
for filename in args:
@@ -17,8 +17,8 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#ifndef CONFIG_DJGPP_LANGINFO_H
-#define CONFIG_DJGPP_LANGINFO_H
+#ifndef GDB_CONFIG_DJGPP_LANGINFO_H
+#define GDB_CONFIG_DJGPP_LANGINFO_H
#include <nl_types.h>
@@ -32,4 +32,4 @@ enum {
extern char *nl_langinfo (nl_item);
-#endif /* CONFIG_DJGPP_LANGINFO_H */
+#endif /* GDB_CONFIG_DJGPP_LANGINFO_H */
@@ -17,9 +17,9 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#ifndef CONFIG_DJGPP_NL_TYPES_H
-#define CONFIG_DJGPP_NL_TYPES_H
+#ifndef GDB_CONFIG_DJGPP_NL_TYPES_H
+#define GDB_CONFIG_DJGPP_NL_TYPES_H
typedef int nl_item;
-#endif /* CONFIG_DJGPP_NL_TYPES_H */
+#endif /* GDB_CONFIG_DJGPP_NL_TYPES_H */
@@ -16,8 +16,8 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#ifndef CONFIG_I386_NM_I386GNU_H
-#define CONFIG_I386_NM_I386GNU_H
+#ifndef GDB_CONFIG_I386_NM_X86_GNU_H
+#define GDB_CONFIG_I386_NM_X86_GNU_H
/* Thread flavors used in re-setting the T bit. */
#define THREAD_STATE_FLAVOR i386_REGS_SEGS_STATE
@@ -34,4 +34,4 @@
((((struct i386_thread_state *) (state))->efl &= ~0x100), 1)
#endif /* __x86_64__ */
-#endif /* CONFIG_I386_NM_I386GNU_H */
+#endif /* GDB_CONFIG_I386_NM_X86_GNU_H */
@@ -17,10 +17,10 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#ifndef CONFIG_SPARC_NM_SOL2_H
-#define CONFIG_SPARC_NM_SOL2_H
+#ifndef GDB_CONFIG_SPARC_NM_SOL2_H
+#define GDB_CONFIG_SPARC_NM_SOL2_H
#define GDB_GREGSET_T prgregset_t
#define GDB_FPREGSET_T prfpregset_t
-#endif /* CONFIG_SPARC_NM_SOL2_H */
+#endif /* GDB_CONFIG_SPARC_NM_SOL2_H */
@@ -17,8 +17,8 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#ifndef PYTHON_PY_COLOR_H
-#define PYTHON_PY_COLOR_H
+#ifndef GDB_PYTHON_PY_COLOR_H
+#define GDB_PYTHON_PY_COLOR_H
#include "python-internal.h"
#include "ui-style.h"
@@ -32,4 +32,4 @@ extern bool gdbpy_is_color (PyObject *obj);
/* Extracts value from OBJ object of gdb.Color type. */
extern const ui_file_style::color &gdbpy_get_color (PyObject *obj);
-#endif /* PYTHON_PY_COLOR_H */
+#endif /* GDB_PYTHON_PY_COLOR_H */
@@ -17,8 +17,8 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-#ifndef SCOPED_SIGNAL_HANDLER_H
-#define SCOPED_SIGNAL_HANDLER_H
+#ifndef GDBSUPPORT_SCOPED_SIGNAL_HANDLER_H
+#define GDBSUPPORT_SCOPED_SIGNAL_HANDLER_H
#include <signal.h>
@@ -70,4 +70,4 @@ class scoped_signal_handler
#endif
};
-#endif /* SCOPED_SIGNAL_HANDLER_H */
+#endif /* GDBSUPPORT_SCOPED_SIGNAL_HANDLER_H */