* [PATCH v8 0/6] Update gnulib version and drop most gnulib patches
@ 2022-03-02 19:08 Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 1/6] Use visual indentation in config.h.in Robbie Harwood
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
Changes this version:
- Reorder last two commits so that warning fixes come after the change that
introduces them.
- Fix comment formatting to comply with grub2 style.
No functional code changes.
Be well,
--Robbie
Robbie Harwood (6):
Use visual indentation in config.h.in
Where present, ensure config-util.h precedes config.h
Drop gnulib fix-base64.patch
Drop gnulib no-abort.patch
Update gnulib version and drop most gnulib patches
Handle warnings introduced by updated gnulib
INSTALL | 4 +-
bootstrap | 319 ++++++++++--------
bootstrap.conf | 23 +-
conf/Makefile.extra-dist | 8 -
config.h.in | 142 ++++++--
configure.ac | 2 +-
grub-core/Makefile.core.def | 3 +
grub-core/disk/host.c | 2 +-
grub-core/disk/luks2.c | 4 +-
grub-core/gensymlist.sh | 1 +
grub-core/kern/emu/argp_common.c | 2 +-
grub-core/kern/emu/main.c | 2 +-
grub-core/lib/gnulib-patches/fix-base64.patch | 21 --
.../lib/gnulib-patches/fix-null-deref.patch | 13 -
.../gnulib-patches/fix-null-state-deref.patch | 12 -
.../fix-regcomp-uninit-token.patch | 15 -
.../fix-regexec-null-deref.patch | 12 -
.../gnulib-patches/fix-uninit-structure.patch | 11 -
.../lib/gnulib-patches/fix-unused-value.patch | 14 -
grub-core/lib/gnulib-patches/no-abort.patch | 26 --
grub-core/lib/posix_wrap/limits.h | 6 +-
grub-core/lib/posix_wrap/sys/types.h | 7 +-
grub-core/lib/xzembed/xz.h | 5 +-
grub-core/osdep/aros/config.c | 2 +-
grub-core/osdep/basic/emunet.c | 2 +-
grub-core/osdep/basic/init.c | 2 +-
grub-core/osdep/haiku/getroot.c | 2 +-
grub-core/osdep/linux/emunet.c | 2 +-
grub-core/osdep/unix/config.c | 2 +-
grub-core/osdep/unix/cputime.c | 2 +-
grub-core/osdep/unix/dl.c | 2 +-
grub-core/osdep/unix/emuconsole.c | 2 +-
grub-core/osdep/unix/getroot.c | 2 +-
grub-core/osdep/windows/config.c | 2 +-
grub-core/osdep/windows/cputime.c | 2 +-
grub-core/osdep/windows/dl.c | 2 +-
grub-core/osdep/windows/emuconsole.c | 2 +-
grub-core/osdep/windows/init.c | 2 +-
include/grub/compiler.h | 4 +-
include/grub/list.h | 2 +-
40 files changed, 347 insertions(+), 343 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
--
2.34.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v8 1/6] Use visual indentation in config.h.in
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
@ 2022-03-02 19:08 ` Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 2/6] Where present, ensure config-util.h precedes config.h Robbie Harwood
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
config.h.in | 56 ++++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/config.h.in b/config.h.in
index 9e8f9911b..1c8c05b9c 100644
--- a/config.h.in
+++ b/config.h.in
@@ -22,46 +22,46 @@
#define MINILZO_CFG_SKIP_LZO1X_DECOMPRESS 1
#if defined (GRUB_BUILD)
-#undef ENABLE_NLS
-#define BUILD_SIZEOF_LONG @BUILD_SIZEOF_LONG@
-#define BUILD_SIZEOF_VOID_P @BUILD_SIZEOF_VOID_P@
-#if defined __APPLE__
-# if defined __BIG_ENDIAN__
-# define BUILD_WORDS_BIGENDIAN 1
-# else
-# define BUILD_WORDS_BIGENDIAN 0
-# endif
-#else
-#define BUILD_WORDS_BIGENDIAN @BUILD_WORDS_BIGENDIAN@
-#endif
+# undef ENABLE_NLS
+# define BUILD_SIZEOF_LONG @BUILD_SIZEOF_LONG@
+# define BUILD_SIZEOF_VOID_P @BUILD_SIZEOF_VOID_P@
+# if defined __APPLE__
+# if defined __BIG_ENDIAN__
+# define BUILD_WORDS_BIGENDIAN 1
+# else
+# define BUILD_WORDS_BIGENDIAN 0
+# endif
+# else /* !defined __APPLE__ */
+# define BUILD_WORDS_BIGENDIAN @BUILD_WORDS_BIGENDIAN@
+# endif /* !defined __APPLE__ */
#elif defined (GRUB_UTIL) || !defined (GRUB_MACHINE)
-#include <config-util.h>
-#else
-#define HAVE_FONT_SOURCE @HAVE_FONT_SOURCE@
+# include <config-util.h>
+#else /* !defined GRUB_UTIL && defined GRUB_MACHINE */
+# define HAVE_FONT_SOURCE @HAVE_FONT_SOURCE@
/* Define if C symbols get an underscore after compilation. */
-#define HAVE_ASM_USCORE @HAVE_ASM_USCORE@
+# define HAVE_ASM_USCORE @HAVE_ASM_USCORE@
/* Define it to one of __bss_start, edata and _edata. */
-#define BSS_START_SYMBOL @BSS_START_SYMBOL@
+# define BSS_START_SYMBOL @BSS_START_SYMBOL@
/* Define it to either end or _end. */
-#define END_SYMBOL @END_SYMBOL@
+# define END_SYMBOL @END_SYMBOL@
/* Name of package. */
-#define PACKAGE "@PACKAGE@"
+# define PACKAGE "@PACKAGE@"
/* Version number of package. */
-#define VERSION "@VERSION@"
+# define VERSION "@VERSION@"
/* Define to the full name and version of this package. */
-#define PACKAGE_STRING "@PACKAGE_STRING@"
+# define PACKAGE_STRING "@PACKAGE_STRING@"
/* Define to the version of this package. */
-#define PACKAGE_VERSION "@PACKAGE_VERSION@"
+# define PACKAGE_VERSION "@PACKAGE_VERSION@"
/* Define to the full name of this package. */
-#define PACKAGE_NAME "@PACKAGE_NAME@"
+# define PACKAGE_NAME "@PACKAGE_NAME@"
/* Define to the address where bug reports for this package should be sent. */
-#define PACKAGE_BUGREPORT "@PACKAGE_BUGREPORT@"
+# define PACKAGE_BUGREPORT "@PACKAGE_BUGREPORT@"
-#define GRUB_TARGET_CPU "@GRUB_TARGET_CPU@"
-#define GRUB_PLATFORM "@GRUB_PLATFORM@"
+# define GRUB_TARGET_CPU "@GRUB_TARGET_CPU@"
+# define GRUB_PLATFORM "@GRUB_PLATFORM@"
-#define RE_ENABLE_I18N 1
+# define RE_ENABLE_I18N 1
-#define _GNU_SOURCE 1
+# define _GNU_SOURCE 1
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 2/6] Where present, ensure config-util.h precedes config.h
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 1/6] Use visual indentation in config.h.in Robbie Harwood
@ 2022-03-02 19:08 ` Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 3/6] Drop gnulib fix-base64.patch Robbie Harwood
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
gnulib defines go in config-util.h, and we need to know whether to
provide duplicates in config.h or not.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
grub-core/disk/host.c | 2 +-
grub-core/gensymlist.sh | 1 +
grub-core/kern/emu/argp_common.c | 2 +-
grub-core/kern/emu/main.c | 2 +-
grub-core/osdep/aros/config.c | 2 +-
grub-core/osdep/basic/emunet.c | 2 +-
grub-core/osdep/basic/init.c | 2 +-
grub-core/osdep/haiku/getroot.c | 2 +-
grub-core/osdep/linux/emunet.c | 2 +-
grub-core/osdep/unix/config.c | 2 +-
grub-core/osdep/unix/cputime.c | 2 +-
grub-core/osdep/unix/dl.c | 2 +-
grub-core/osdep/unix/emuconsole.c | 2 +-
grub-core/osdep/unix/getroot.c | 2 +-
grub-core/osdep/windows/config.c | 2 +-
grub-core/osdep/windows/cputime.c | 2 +-
grub-core/osdep/windows/dl.c | 2 +-
grub-core/osdep/windows/emuconsole.c | 2 +-
grub-core/osdep/windows/init.c | 2 +-
19 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/grub-core/disk/host.c b/grub-core/disk/host.c
index c151d225d..f34529f86 100644
--- a/grub-core/disk/host.c
+++ b/grub-core/disk/host.c
@@ -20,8 +20,8 @@
/* When using the disk, make a reference to this module. Otherwise
the user will end up with a useless module :-). */
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/dl.h>
#include <grub/disk.h>
diff --git a/grub-core/gensymlist.sh b/grub-core/gensymlist.sh
index 5beaabdd6..5074ef6aa 100644
--- a/grub-core/gensymlist.sh
+++ b/grub-core/gensymlist.sh
@@ -31,6 +31,7 @@ cat <<EOF
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <../config-util.h>
EOF
for i in $*; do
diff --git a/grub-core/kern/emu/argp_common.c b/grub-core/kern/emu/argp_common.c
index 166885870..8cb4608c3 100644
--- a/grub-core/kern/emu/argp_common.c
+++ b/grub-core/kern/emu/argp_common.c
@@ -17,8 +17,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#pragma GCC diagnostic ignored "-Wmissing-prototypes"
#pragma GCC diagnostic ignored "-Wmissing-declarations"
diff --git a/grub-core/kern/emu/main.c b/grub-core/kern/emu/main.c
index 425bb9603..dbbbffb5c 100644
--- a/grub-core/kern/emu/main.c
+++ b/grub-core/kern/emu/main.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <time.h>
#include <stdio.h>
diff --git a/grub-core/osdep/aros/config.c b/grub-core/osdep/aros/config.c
index c82d0ea8e..55f5728ef 100644
--- a/grub-core/osdep/aros/config.c
+++ b/grub-core/osdep/aros/config.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/emu/hostdisk.h>
#include <grub/emu/exec.h>
diff --git a/grub-core/osdep/basic/emunet.c b/grub-core/osdep/basic/emunet.c
index 6362e5cfb..dbfd316d6 100644
--- a/grub-core/osdep/basic/emunet.c
+++ b/grub-core/osdep/basic/emunet.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/i18n.h>
#include <grub/emu/net.h>
diff --git a/grub-core/osdep/basic/init.c b/grub-core/osdep/basic/init.c
index c54c710db..b104c7e16 100644
--- a/grub-core/osdep/basic/init.c
+++ b/grub-core/osdep/basic/init.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/util/misc.h>
#include <grub/i18n.h>
diff --git a/grub-core/osdep/haiku/getroot.c b/grub-core/osdep/haiku/getroot.c
index 4e123c090..927a1ebc9 100644
--- a/grub-core/osdep/haiku/getroot.c
+++ b/grub-core/osdep/haiku/getroot.c
@@ -1,5 +1,5 @@
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <string.h>
diff --git a/grub-core/osdep/linux/emunet.c b/grub-core/osdep/linux/emunet.c
index 19b188f09..d5a641735 100644
--- a/grub-core/osdep/linux/emunet.c
+++ b/grub-core/osdep/linux/emunet.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <sys/socket.h>
#include <sys/types.h>
diff --git a/grub-core/osdep/unix/config.c b/grub-core/osdep/unix/config.c
index 7d6325138..0b1f7618d 100644
--- a/grub-core/osdep/unix/config.c
+++ b/grub-core/osdep/unix/config.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/emu/hostdisk.h>
#include <grub/emu/exec.h>
diff --git a/grub-core/osdep/unix/cputime.c b/grub-core/osdep/unix/cputime.c
index cff359a3b..fb6ff55a1 100644
--- a/grub-core/osdep/unix/cputime.c
+++ b/grub-core/osdep/unix/cputime.c
@@ -1,5 +1,5 @@
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <sys/times.h>
#include <unistd.h>
diff --git a/grub-core/osdep/unix/dl.c b/grub-core/osdep/unix/dl.c
index 562b101a2..99b189bc1 100644
--- a/grub-core/osdep/unix/dl.c
+++ b/grub-core/osdep/unix/dl.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/dl.h>
#include <grub/misc.h>
diff --git a/grub-core/osdep/unix/emuconsole.c b/grub-core/osdep/unix/emuconsole.c
index 7308798ef..cac159424 100644
--- a/grub-core/osdep/unix/emuconsole.c
+++ b/grub-core/osdep/unix/emuconsole.c
@@ -17,8 +17,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/term.h>
#include <grub/types.h>
diff --git a/grub-core/osdep/unix/getroot.c b/grub-core/osdep/unix/getroot.c
index 74f69116d..c94cc0554 100644
--- a/grub-core/osdep/unix/getroot.c
+++ b/grub-core/osdep/unix/getroot.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config-util.h>
#include <config.h>
+#include <config-util.h>
#include <sys/stat.h>
#include <sys/types.h>
diff --git a/grub-core/osdep/windows/config.c b/grub-core/osdep/windows/config.c
index 928ab1a49..2bb8a2fd8 100644
--- a/grub-core/osdep/windows/config.c
+++ b/grub-core/osdep/windows/config.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/emu/hostfile.h>
#include <grub/emu/config.h>
diff --git a/grub-core/osdep/windows/cputime.c b/grub-core/osdep/windows/cputime.c
index 3568aa2d3..5d06d79dd 100644
--- a/grub-core/osdep/windows/cputime.c
+++ b/grub-core/osdep/windows/cputime.c
@@ -1,5 +1,5 @@
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/emu/misc.h>
#include <windows.h>
diff --git a/grub-core/osdep/windows/dl.c b/grub-core/osdep/windows/dl.c
index eec6a24ad..8eab7057e 100644
--- a/grub-core/osdep/windows/dl.c
+++ b/grub-core/osdep/windows/dl.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/dl.h>
#include <grub/misc.h>
diff --git a/grub-core/osdep/windows/emuconsole.c b/grub-core/osdep/windows/emuconsole.c
index 4fb3693cc..17a44de46 100644
--- a/grub-core/osdep/windows/emuconsole.c
+++ b/grub-core/osdep/windows/emuconsole.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/term.h>
#include <grub/misc.h>
diff --git a/grub-core/osdep/windows/init.c b/grub-core/osdep/windows/init.c
index 6297de632..51a9647dd 100644
--- a/grub-core/osdep/windows/init.c
+++ b/grub-core/osdep/windows/init.c
@@ -16,8 +16,8 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <config.h>
#include <config-util.h>
+#include <config.h>
#include <grub/util/misc.h>
#include <grub/osdep/hostfile.h>
#include <grub/util/windows.h>
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 3/6] Drop gnulib fix-base64.patch
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 1/6] Use visual indentation in config.h.in Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 2/6] Where present, ensure config-util.h precedes config.h Robbie Harwood
@ 2022-03-02 19:08 ` Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 4/6] Drop gnulib no-abort.patch Robbie Harwood
` (4 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
Originally added in 9fbdec2f6b4fa8b549daa4d49134d1fe89d95ef9 and
subsequently modified in 552c9fd08122a3036c724ce96dfe68aa2f75705f,
fix-base64.patch handled two problems we have using gnulib, which are
exerciesd by the base64 module but not directly caused by it.
First, grub2 defines its own bool type, while gnulib expects the
equivalent of stdbool.h to be present. Rather than patching gnulib,
instead use gnulib's stdbool module to provide a bool type if needed.
(Suggested by Simon Josefsson.)
Second, our config.h doesn't always inherit config-util.h, which is
where gnulib-related options like _GL_ATTRIBUTE_CONST end up.
fix-base64.h worked around this by defining the attribute away, but this
workaround is better placed in config.h itself, not a gnulib patch.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
bootstrap.conf | 3 ++-
| 1 -
config.h.in | 4 ++++
grub-core/lib/gnulib-patches/fix-base64.patch | 21 -------------------
grub-core/lib/posix_wrap/sys/types.h | 7 +++----
grub-core/lib/xzembed/xz.h | 5 +----
6 files changed, 10 insertions(+), 31 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 0dd893c5c..21a8cf15d 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -35,6 +35,7 @@ gnulib_modules="
realloc-gnu
regex
save-cwd
+ stdbool
"
gnulib_tool_option_extras="\
@@ -80,7 +81,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-base64 fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
+ for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
--git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 8f1485d52..15a9b74e9 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -28,7 +28,6 @@ EXTRA_DIST += grub-core/gensymlist.sh
EXTRA_DIST += grub-core/genemuinit.sh
EXTRA_DIST += grub-core/genemuinitheader.sh
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-base64.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-state-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
diff --git a/config.h.in b/config.h.in
index 1c8c05b9c..965eaffce 100644
--- a/config.h.in
+++ b/config.h.in
@@ -64,4 +64,8 @@
# define _GNU_SOURCE 1
+# ifndef _GL_INLINE_HEADER_BEGIN
+# define _GL_ATTRIBUTE_CONST __attribute__ ((const))
+# endif /* !_GL_INLINE_HEADER_BEGIN */
+
#endif
diff --git a/grub-core/lib/gnulib-patches/fix-base64.patch b/grub-core/lib/gnulib-patches/fix-base64.patch
deleted file mode 100644
index 985db1279..000000000
--- a/grub-core/lib/gnulib-patches/fix-base64.patch
+++ /dev/null
@@ -1,21 +0,0 @@
-diff --git a/lib/base64.h b/lib/base64.h
-index 9cd0183b8..185a2afa1 100644
---- a/lib/base64.h
-+++ b/lib/base64.h
-@@ -21,8 +21,14 @@
- /* Get size_t. */
- # include <stddef.h>
-
--/* Get bool. */
--# include <stdbool.h>
-+#ifndef GRUB_POSIX_BOOL_DEFINED
-+typedef enum { false = 0, true = 1 } bool;
-+#define GRUB_POSIX_BOOL_DEFINED 1
-+#endif
-+
-+#ifndef _GL_ATTRIBUTE_CONST
-+# define _GL_ATTRIBUTE_CONST /* empty */
-+#endif
-
- # ifdef __cplusplus
- extern "C" {
diff --git a/grub-core/lib/posix_wrap/sys/types.h b/grub-core/lib/posix_wrap/sys/types.h
index 854eb0122..eeda543c4 100644
--- a/grub-core/lib/posix_wrap/sys/types.h
+++ b/grub-core/lib/posix_wrap/sys/types.h
@@ -23,11 +23,10 @@
#include <stddef.h>
+/* Provided by gnulib if not present. */
+#include <stdbool.h>
+
typedef grub_ssize_t ssize_t;
-#ifndef GRUB_POSIX_BOOL_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-#define GRUB_POSIX_BOOL_DEFINED 1
-#endif
typedef grub_uint8_t uint8_t;
typedef grub_uint16_t uint16_t;
diff --git a/grub-core/lib/xzembed/xz.h b/grub-core/lib/xzembed/xz.h
index f7b32d800..d1417039a 100644
--- a/grub-core/lib/xzembed/xz.h
+++ b/grub-core/lib/xzembed/xz.h
@@ -29,10 +29,7 @@
#include <unistd.h>
#include <string.h>
#include <grub/misc.h>
-
-#ifndef GRUB_POSIX_BOOL_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-#endif
+#include <stdbool.h>
/**
* enum xz_ret - Return codes
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
` (2 preceding siblings ...)
2022-03-02 19:08 ` [PATCH v8 3/6] Drop gnulib fix-base64.patch Robbie Harwood
@ 2022-03-02 19:08 ` Robbie Harwood
2022-03-03 0:22 ` Glenn Washburn
2022-03-02 19:08 ` [PATCH v8 5/6] Update gnulib version and drop most gnulib patches Robbie Harwood
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
Originally added in db7337a3d353a817ffe9eb4a3702120527100be9, this
patched out all relevant invocations of abort() in gnulib. While it was
not documented why at the time, testing suggests that there's no abort()
implementation available for gnulib to use.
gnulib's position is that the use of abort() is correct here, since it
happens when input violates a "shall" from POSIX. Additionally, the
code in question is probably not reachable. Since abort() is more
friendly to user-space, they prefer to make no change, so we can just
carry a define instead. (Suggested by Paul Eggert.)
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
bootstrap.conf | 2 +-
| 1 -
config.h.in | 10 ++++++++
grub-core/lib/gnulib-patches/no-abort.patch | 26 ---------------------
4 files changed, 11 insertions(+), 28 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
diff --git a/bootstrap.conf b/bootstrap.conf
index 21a8cf15d..71acbeeb1 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -82,7 +82,7 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
- fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
+ fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width; do
patch -d grub-core/lib/gnulib -p2 \
< "grub-core/lib/gnulib-patches/$patchname.patch"
done
--git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 15a9b74e9..4ddc3c8f7 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -35,7 +35,6 @@ EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
EXTRA_DIST += grub-core/lib/libgcrypt
EXTRA_DIST += grub-core/lib/libgcrypt-grub/mpi/generic
diff --git a/config.h.in b/config.h.in
index 965eaffce..209e27449 100644
--- a/config.h.in
+++ b/config.h.in
@@ -66,6 +66,16 @@
# ifndef _GL_INLINE_HEADER_BEGIN
# define _GL_ATTRIBUTE_CONST __attribute__ ((const))
+
+/*
+ * We don't have an abort() for gnulib to call in regexp. Because gnulib is
+ * built as a separate object that is then linked with, it doesn't have any
+ * of our headers or functions around to use - so we unfortunately can't
+ * print anything. Builds of emu include the system's stdlib, which includes
+ * a prototype for abort(), so leave this as a macro that doesn't take
+ * arguments.
+ */
+# define abort __builtin_trap
# endif /* !_GL_INLINE_HEADER_BEGIN */
#endif
diff --git a/grub-core/lib/gnulib-patches/no-abort.patch b/grub-core/lib/gnulib-patches/no-abort.patch
deleted file mode 100644
index e469c4762..000000000
--- a/grub-core/lib/gnulib-patches/no-abort.patch
+++ /dev/null
@@ -1,26 +0,0 @@
-diff --git a/lib/regcomp.c b/lib/regcomp.c
-index cc85f35ac..de45ebb5c 100644
---- a/lib/regcomp.c
-+++ b/lib/regcomp.c
-@@ -528,9 +528,9 @@ regerror (int errcode, const regex_t *__restrict preg, char *__restrict errbuf,
- to this routine. If we are given anything else, or if other regex
- code generates an invalid error code, then the program has a bug.
- Dump core so we can fix it. */
-- abort ();
--
-- msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
-+ msg = gettext ("unknown regexp error");
-+ else
-+ msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
-
- msg_size = strlen (msg) + 1; /* Includes the null. */
-
-@@ -1136,7 +1136,7 @@ optimize_utf8 (re_dfa_t *dfa)
- }
- break;
- default:
-- abort ();
-+ break;
- }
-
- if (mb_chars || has_period)
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 5/6] Update gnulib version and drop most gnulib patches
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
` (3 preceding siblings ...)
2022-03-02 19:08 ` [PATCH v8 4/6] Drop gnulib no-abort.patch Robbie Harwood
@ 2022-03-02 19:08 ` Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 6/6] Handle warnings introduced by updated gnulib Robbie Harwood
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
In addition to the changes carried in our gnulib patches, several
Coverity and code hygiene fixes that were previously downstream are also
included in this 3-year gnulib increment.
Unfortunately, fix-width.patch is retained.
Bump minimum autoconf version from 2.63 to 2.64 and automake from 1.11
to 1.14, as required by gnulib.
Sync bootstrap script itself with gnulib.
Update regexp module for new dynarray dependency.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
INSTALL | 4 +-
bootstrap | 319 ++++++++++--------
bootstrap.conf | 22 +-
| 6 -
config.h.in | 67 ++++
configure.ac | 2 +-
grub-core/Makefile.core.def | 3 +
.../lib/gnulib-patches/fix-null-deref.patch | 13 -
.../gnulib-patches/fix-null-state-deref.patch | 12 -
.../fix-regcomp-uninit-token.patch | 15 -
.../fix-regexec-null-deref.patch | 12 -
.../gnulib-patches/fix-uninit-structure.patch | 11 -
.../lib/gnulib-patches/fix-unused-value.patch | 14 -
13 files changed, 266 insertions(+), 234 deletions(-)
delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
diff --git a/INSTALL b/INSTALL
index a64f63723..6aee8a120 100644
--- a/INSTALL
+++ b/INSTALL
@@ -47,8 +47,8 @@ If you use a development snapshot or want to hack on GRUB you may
need the following.
* Python 3 (NOTE: python 2.6 should still work, but it's not tested)
-* Autoconf 2.63 or later
-* Automake 1.11 or later
+* Autoconf 2.64 or later
+* Automake 1.14 or later
Your distro may package cross-compiling toolchains such as the following
incomplete list on Debian: gcc-aarch64-linux-gnu, gcc-arm-linux-gnueabihf,
diff --git a/bootstrap b/bootstrap
index 5b08e7e2d..dc2238f4a 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,10 +1,10 @@
#! /bin/sh
# Print a version string.
-scriptversion=2019-01-04.17; # UTC
+scriptversion=2022-01-26.05; # UTC
# Bootstrap this package from checked-out sources.
-# Copyright (C) 2003-2019 Free Software Foundation, Inc.
+# Copyright (C) 2003-2022 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -47,7 +47,7 @@ PERL="${PERL-perl}"
me=$0
-default_gnulib_url=git://git.sv.gnu.org/gnulib
+default_gnulib_url=https://git.savannah.gnu.org/git/gnulib.git
usage() {
cat <<EOF
@@ -71,7 +71,9 @@ Options:
--no-git do not use git to update gnulib. Requires that
--gnulib-srcdir point to a correct gnulib snapshot
--skip-po do not download po files
-
+EOF
+ bootstrap_print_option_usage_hook
+ cat <<EOF
If the file $me.conf exists in the same directory as this script, its
contents are read as shell variables to configure the bootstrap.
@@ -113,6 +115,12 @@ Running without arguments will suffice in most cases.
EOF
}
+copyright_year=`echo "$scriptversion" | sed -e 's/[^0-9].*//'`
+copyright="Copyright (C) ${copyright_year} Free Software Foundation, Inc.
+License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
+This is free software: you are free to change and redistribute it.
+There is NO WARRANTY, to the extent permitted by law."
+
# warnf_ FORMAT-STRING ARG1...
warnf_ ()
{
@@ -154,6 +162,18 @@ gnulib_files=
: ${AUTOPOINT=autopoint}
: ${AUTORECONF=autoreconf}
+# A function to be called for each unrecognized option. Returns 0 if
+# the option in $1 has been processed by the function. Returns 1 if
+# the option has not been processed by the function. Override it via
+# your own definition in bootstrap.conf
+
+bootstrap_option_hook() { return 1; }
+
+# A function to be called in order to print the --help information
+# corresponding to user-defined command-line options.
+
+bootstrap_print_option_usage_hook() { :; }
+
# A function to be called right after gnulib-tool is run.
# Override it via your own definition in bootstrap.conf.
bootstrap_post_import_hook() { :; }
@@ -166,11 +186,11 @@ bootstrap_epilogue() { :; }
# specified directory. Fill in the first %s with the destination
# directory and the second with the domain name.
po_download_command_format=\
-"wget --mirror --level=1 -nd -q -A.po -P '%s' \
+"wget --mirror --level=1 -nd -nv -A.po -P '%s' \
https://translationproject.org/latest/%s/"
# Prefer a non-empty tarname (4th argument of AC_INIT if given), else
-# fall back to the package name (1st argument with munging)
+# fall back to the package name (1st argument with munging).
extract_package_name='
/^AC_INIT(\[*/{
s///
@@ -187,8 +207,11 @@ extract_package_name='
p
}
'
-package=$(sed -n "$extract_package_name" configure.ac) \
- || die 'cannot find package name in configure.ac'
+package=$(${AUTOCONF:-autoconf} --trace AC_INIT:\$4 configure.ac 2>/dev/null)
+if test -z "$package"; then
+ package=$(sed -n "$extract_package_name" configure.ac) \
+ || die 'cannot find package name in configure.ac'
+fi
gnulib_name=lib$package
build_aux=build-aux
@@ -290,62 +313,6 @@ find_tool ()
eval "export $find_tool_envvar"
}
-# Override the default configuration, if necessary.
-# Make sure that bootstrap.conf is sourced from the current directory
-# if we were invoked as "sh bootstrap".
-case "$0" in
- */*) test -r "$0.conf" && . "$0.conf" ;;
- *) test -r "$0.conf" && . ./"$0.conf" ;;
-esac
-
-if test "$vc_ignore" = auto; then
- vc_ignore=
- test -d .git && vc_ignore=.gitignore
- test -d CVS && vc_ignore="$vc_ignore .cvsignore"
-fi
-
-if test x"$gnulib_modules$gnulib_files$gnulib_extra_files" = x; then
- use_gnulib=false
-else
- use_gnulib=true
-fi
-
-# Translate configuration into internal form.
-
-# Parse options.
-
-for option
-do
- case $option in
- --help)
- usage
- exit;;
- --gnulib-srcdir=*)
- GNULIB_SRCDIR=${option#--gnulib-srcdir=};;
- --skip-po)
- SKIP_PO=t;;
- --force)
- checkout_only_file=;;
- --copy)
- copy=true;;
- --bootstrap-sync)
- bootstrap_sync=true;;
- --no-bootstrap-sync)
- bootstrap_sync=false;;
- --no-git)
- use_git=false;;
- *)
- die "$option: unknown option";;
- esac
-done
-
-$use_git || test -d "$GNULIB_SRCDIR" \
- || die "Error: --no-git requires --gnulib-srcdir"
-
-if test -n "$checkout_only_file" && test ! -r "$checkout_only_file"; then
- die "Bootstrapping from a non-checked-out distribution is risky."
-fi
-
# Strip blank and comment lines to leave significant entries.
gitignore_entries() {
sed '/^#/d; /^$/d' "$@"
@@ -387,6 +354,137 @@ insert_vc_ignore() {
insert_if_absent "$vc_ignore_file" "$pattern"
}
+symlink_to_dir()
+{
+ src=$1/$2
+ dst=${3-$2}
+
+ test -f "$src" && {
+
+ # If the destination directory doesn't exist, create it.
+ # This is required at least for "lib/uniwidth/cjk.h".
+ dst_dir=$(dirname "$dst")
+ if ! test -d "$dst_dir"; then
+ mkdir -p "$dst_dir"
+
+ # If we've just created a directory like lib/uniwidth,
+ # tell version control system(s) it's ignorable.
+ # FIXME: for now, this does only one level
+ parent=$(dirname "$dst_dir")
+ for dot_ig in x $vc_ignore; do
+ test $dot_ig = x && continue
+ ig=$parent/$dot_ig
+ insert_vc_ignore $ig "${dst_dir##*/}"
+ done
+ fi
+
+ if $copy; then
+ {
+ test ! -h "$dst" || {
+ echo "$me: rm -f $dst" &&
+ rm -f "$dst"
+ }
+ } &&
+ test -f "$dst" &&
+ cmp -s "$src" "$dst" || {
+ echo "$me: cp -fp $src $dst" &&
+ cp -fp "$src" "$dst"
+ }
+ else
+ # Leave any existing symlink alone, if it already points to the source,
+ # so that broken build tools that care about symlink times
+ # aren't confused into doing unnecessary builds. Conversely, if the
+ # existing symlink's timestamp is older than the source, make it afresh,
+ # so that broken tools aren't confused into skipping needed builds. See
+ # <https://lists.gnu.org/r/bug-gnulib/2011-05/msg00326.html>.
+ test -h "$dst" &&
+ src_ls=$(ls -diL "$src" 2>/dev/null) && set $src_ls && src_i=$1 &&
+ dst_ls=$(ls -diL "$dst" 2>/dev/null) && set $dst_ls && dst_i=$1 &&
+ test "$src_i" = "$dst_i" &&
+ both_ls=$(ls -dt "$src" "$dst") &&
+ test "X$both_ls" = "X$dst$nl$src" || {
+ dot_dots=
+ case $src in
+ /*) ;;
+ *)
+ case /$dst/ in
+ *//* | */../* | */./* | /*/*/*/*/*/)
+ die "invalid symlink calculation: $src -> $dst";;
+ /*/*/*/*/) dot_dots=../../../;;
+ /*/*/*/) dot_dots=../../;;
+ /*/*/) dot_dots=../;;
+ esac;;
+ esac
+
+ echo "$me: ln -fs $dot_dots$src $dst" &&
+ ln -fs "$dot_dots$src" "$dst"
+ }
+ fi
+ }
+}
+
+# Override the default configuration, if necessary.
+# Make sure that bootstrap.conf is sourced from the current directory
+# if we were invoked as "sh bootstrap".
+case "$0" in
+ */*) test -r "$0.conf" && . "$0.conf" ;;
+ *) test -r "$0.conf" && . ./"$0.conf" ;;
+esac
+
+if test "$vc_ignore" = auto; then
+ vc_ignore=
+ test -d .git && vc_ignore=.gitignore
+ test -d CVS && vc_ignore="$vc_ignore .cvsignore"
+fi
+
+if test x"$gnulib_modules$gnulib_files$gnulib_extra_files" = x; then
+ use_gnulib=false
+else
+ use_gnulib=true
+fi
+
+# Translate configuration into internal form.
+
+# Parse options.
+
+for option
+do
+ case $option in
+ --help)
+ usage
+ exit;;
+ --version)
+ set -e
+ echo "bootstrap $scriptversion"
+ echo "$copyright"
+ exit 0
+ ;;
+ --gnulib-srcdir=*)
+ GNULIB_SRCDIR=${option#--gnulib-srcdir=};;
+ --skip-po)
+ SKIP_PO=t;;
+ --force)
+ checkout_only_file=;;
+ --copy)
+ copy=true;;
+ --bootstrap-sync)
+ bootstrap_sync=true;;
+ --no-bootstrap-sync)
+ bootstrap_sync=false;;
+ --no-git)
+ use_git=false;;
+ *)
+ bootstrap_option_hook $option || die "$option: unknown option";;
+ esac
+done
+
+$use_git || test -d "$GNULIB_SRCDIR" \
+ || die "Error: --no-git requires --gnulib-srcdir"
+
+if test -n "$checkout_only_file" && test ! -r "$checkout_only_file"; then
+ die "Bootstrapping from a non-checked-out distribution is risky."
+fi
+
# Die if there is no AC_CONFIG_AUX_DIR($build_aux) line in configure.ac.
found_aux_dir=no
grep '^[ ]*AC_CONFIG_AUX_DIR(\['"$build_aux"'\])' configure.ac \
@@ -665,9 +763,25 @@ if $use_gnulib; then
shallow=
if test -z "$GNULIB_REVISION"; then
git clone -h 2>&1 | grep -- --depth > /dev/null && shallow='--depth 2'
+ git clone $shallow ${GNULIB_URL:-$default_gnulib_url} "$gnulib_path" \
+ || cleanup_gnulib
+ else
+ git fetch -h 2>&1 | grep -- --depth > /dev/null && shallow='--depth 2'
+ mkdir -p "$gnulib_path"
+ # Only want a shallow checkout of $GNULIB_REVISION, but git does not
+ # support cloning by commit hash. So attempt a shallow fetch by commit
+ # hash to minimize the amount of data downloaded and changes needed to
+ # be processed, which can drastically reduce download and processing
+ # time for checkout. If the fetch by commit fails, a shallow fetch can
+ # not be performed because we do not know what the depth of the commit
+ # is without fetching all commits. So fallback to fetching all commits.
+ git -C "$gnulib_path" init
+ git -C "$gnulib_path" remote add origin ${GNULIB_URL:-$default_gnulib_url}
+ git -C "$gnulib_path" fetch $shallow origin "$GNULIB_REVISION" \
+ || git -C "$gnulib_path" fetch origin \
+ || cleanup_gnulib
+ git -C "$gnulib_path" reset --hard FETCH_HEAD
fi
- git clone $shallow ${GNULIB_URL:-$default_gnulib_url} "$gnulib_path" \
- || cleanup_gnulib
trap - 1 2 13 15
fi
@@ -784,75 +898,6 @@ case $SKIP_PO in
fi;;
esac
-symlink_to_dir()
-{
- src=$1/$2
- dst=${3-$2}
-
- test -f "$src" && {
-
- # If the destination directory doesn't exist, create it.
- # This is required at least for "lib/uniwidth/cjk.h".
- dst_dir=$(dirname "$dst")
- if ! test -d "$dst_dir"; then
- mkdir -p "$dst_dir"
-
- # If we've just created a directory like lib/uniwidth,
- # tell version control system(s) it's ignorable.
- # FIXME: for now, this does only one level
- parent=$(dirname "$dst_dir")
- for dot_ig in x $vc_ignore; do
- test $dot_ig = x && continue
- ig=$parent/$dot_ig
- insert_vc_ignore $ig "${dst_dir##*/}"
- done
- fi
-
- if $copy; then
- {
- test ! -h "$dst" || {
- echo "$me: rm -f $dst" &&
- rm -f "$dst"
- }
- } &&
- test -f "$dst" &&
- cmp -s "$src" "$dst" || {
- echo "$me: cp -fp $src $dst" &&
- cp -fp "$src" "$dst"
- }
- else
- # Leave any existing symlink alone, if it already points to the source,
- # so that broken build tools that care about symlink times
- # aren't confused into doing unnecessary builds. Conversely, if the
- # existing symlink's timestamp is older than the source, make it afresh,
- # so that broken tools aren't confused into skipping needed builds. See
- # <https://lists.gnu.org/r/bug-gnulib/2011-05/msg00326.html>.
- test -h "$dst" &&
- src_ls=$(ls -diL "$src" 2>/dev/null) && set $src_ls && src_i=$1 &&
- dst_ls=$(ls -diL "$dst" 2>/dev/null) && set $dst_ls && dst_i=$1 &&
- test "$src_i" = "$dst_i" &&
- both_ls=$(ls -dt "$src" "$dst") &&
- test "X$both_ls" = "X$dst$nl$src" || {
- dot_dots=
- case $src in
- /*) ;;
- *)
- case /$dst/ in
- *//* | */../* | */./* | /*/*/*/*/*/)
- die "invalid symlink calculation: $src -> $dst";;
- /*/*/*/*/) dot_dots=../../../;;
- /*/*/*/) dot_dots=../../;;
- /*/*/) dot_dots=../;;
- esac;;
- esac
-
- echo "$me: ln -fs $dot_dots$src $dst" &&
- ln -fs "$dot_dots$src" "$dst"
- }
- fi
- }
-}
-
version_controlled_file() {
parent=$1
file=$2
@@ -970,7 +1015,7 @@ bootstrap_post_import_hook \
# Uninitialized submodules are listed with an initial dash.
if $use_git && git submodule | grep '^-' >/dev/null; then
die "some git submodules are not initialized. " \
- "Run 'git submodule init' and bootstrap again."
+ "Run 'git submodule update --init' and bootstrap again."
fi
# Remove any dangling symlink matching "*.m4" or "*.[ch]" in some
@@ -1064,7 +1109,7 @@ bootstrap_epilogue
echo "$0: done. Now you can run './configure'."
-# Local variables:
+# Local Variables:
# eval: (add-hook 'before-save-hook 'time-stamp)
# time-stamp-start: "scriptversion="
# time-stamp-format: "%:y-%02m-%02d.%02H"
diff --git a/bootstrap.conf b/bootstrap.conf
index 71acbeeb1..64ea9bcb5 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -1,6 +1,6 @@
# Bootstrap configuration.
-# Copyright (C) 2006-2019 Free Software Foundation, Inc.
+# Copyright (C) 2006-2022 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -16,11 +16,10 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.
-GNULIB_REVISION=d271f868a8df9bbec29049d01e056481b7a1a263
+GNULIB_REVISION=9f48fb992a3d7e96610c4ce8be969cff2d61a01b
# gnulib modules used by this package.
-# mbswidth is used by gnulib-fix-width.diff's changes to argp rather than
-# directly.
+# mbswidth is used by fix-width.diff's changes to argp rather than directly.
gnulib_modules="
argp
base64
@@ -67,8 +66,8 @@ SKIP_PO=t
# Build prerequisites
buildreq="\
-autoconf 2.63
-automake 1.11
+autoconf 2.64
+automake 1.14
gettext 0.18.3
git 1.5.5
patch -
@@ -81,11 +80,12 @@ cp -a INSTALL INSTALL.grub
bootstrap_post_import_hook () {
set -e
- for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
- fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width; do
- patch -d grub-core/lib/gnulib -p2 \
- < "grub-core/lib/gnulib-patches/$patchname.patch"
- done
+
+ # Instead of patching our gnulib and therefore maintaining a fork, submit
+ # changes to gnulib and update the hash above when they've merged. Do not
+ # add new patches here.
+ patch -d grub-core/lib/gnulib -p2 < grub-core/lib/gnulib-patches/fix-width.patch
+
for patchname in \
0001-Support-POTFILES-shell \
0002-Handle-gettext_printf-shell-function \
--git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
index 4ddc3c8f7..5e7126f98 100644
--- a/conf/Makefile.extra-dist
+++ b/conf/Makefile.extra-dist
@@ -28,12 +28,6 @@ EXTRA_DIST += grub-core/gensymlist.sh
EXTRA_DIST += grub-core/genemuinit.sh
EXTRA_DIST += grub-core/genemuinitheader.sh
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-deref.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-null-state-deref.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
-EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
EXTRA_DIST += grub-core/lib/libgcrypt
diff --git a/config.h.in b/config.h.in
index 209e27449..d0f4f8609 100644
--- a/config.h.in
+++ b/config.h.in
@@ -65,7 +65,74 @@
# define _GNU_SOURCE 1
# ifndef _GL_INLINE_HEADER_BEGIN
+/*
+ * gnulib gets configured against the host, not the target, and the rest of
+ * our buildsystem works around that. This is difficult to avoid as gnulib's
+ * detection requires a more capable system than our target. Instead, we
+ * reach in and set values appropriately - intentionally setting more than the
+ * bare minimum. If, when updating gnulib, something breaks, there's probably
+ * a change needed here or in grub-core/Makefile.core.def.
+ */
+# define SIZE_MAX ((size_t) -1)
+# define _GL_ATTRIBUTE_ALLOC_SIZE(args) \
+ __attribute__ ((__alloc_size__ args))
+# define _GL_ATTRIBUTE_ALWAYS_INLINE __attribute__ ((__always_inline__))
+# define _GL_ATTRIBUTE_ARTIFICIAL __attribute__ ((__artificial__))
+# define _GL_ATTRIBUTE_COLD __attribute__ ((cold))
# define _GL_ATTRIBUTE_CONST __attribute__ ((const))
+# define _GL_ATTRIBUTE_DEALLOC(f, i) __attribute ((__malloc__ (f, i)))
+# define _GL_ATTRIBUTE_DEALLOC_FREE _GL_ATTRIBUTE_DEALLOC (free, 1)
+# define _GL_ATTRIBUTE_DEPRECATED __attribute__ ((__deprecated__))
+# define _GL_ATTRIBUTE_ERROR(msg) __attribute__ ((__error__ (msg)))
+# define _GL_ATTRIBUTE_EXTERNALLY_VISIBLE \
+ __attribute__ ((externally_visible))
+# define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec))
+# define _GL_ATTRIBUTE_LEAF __attribute__ ((__leaf__))
+# define _GL_ATTRIBUTE_MALLOC __attribute__ ((malloc))
+# define _GL_ATTRIBUTE_MAYBE_UNUSED _GL_ATTRIBUTE_UNUSED
+# define _GL_ATTRIBUTE_MAY_ALIAS __attribute__ ((__may_alias__))
+# define _GL_ATTRIBUTE_NODISCARD __attribute__ ((__warn_unused_result__))
+# define _GL_ATTRIBUTE_NOINLINE __attribute__ ((__noinline__))
+# define _GL_ATTRIBUTE_NONNULL(args) __attribute__ ((__nonnull__ args))
+# define _GL_ATTRIBUTE_NONSTRING __attribute__ ((__nonstring__))
+# define _GL_ATTRIBUTE_PACKED __attribute__ ((__packed__))
+# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
+# define _GL_ATTRIBUTE_RETURNS_NONNULL \
+ __attribute__ ((__returns_nonnull__))
+# define _GL_ATTRIBUTE_SENTINEL(pos) __attribute__ ((__sentinel__ pos))
+# define _GL_ATTRIBUTE_UNUSED __attribute__ ((__unused__))
+# define _GL_ATTRIBUTE_WARNING(msg) __attribute__ ((__warning__ (msg)))
+# define _GL_CMP(n1, n2) (((n1) > (n2)) - ((n1) < (n2)))
+# define _GL_GNUC_PREREQ GNUC_PREREQ
+# define _GL_INLINE inline
+# define _GL_UNUSED_LABEL _GL_ATTRIBUTE_UNUSED
+
+/* We can't use __has_attribute for these because gcc-5.1 is too old for
+ * that. Everything above is present in that version, though. */
+# if __GNUC__ >= 7
+# define _GL_ATTRIBUTE_FALLTHROUGH __attribute__ ((fallthrough))
+# else
+# define _GL_ATTRIBUTE_FALLTHROUGH /* empty */
+# endif
+
+# ifndef ASM_FILE
+typedef __INT_FAST32_TYPE__ int_fast32_t;
+typedef __UINT_FAST32_TYPE__ uint_fast32_t;
+# endif
+
+/* Ensure ialloc nests static/non-static inline properly. */
+# define IALLOC_INLINE static inline
+
+/*
+ * gnulib uses these for blocking out warnings they can't/won't fix. gnulib
+ * also makes the decision about whether to provide a declaration for
+ * reallocarray() at compile-time, so this is a convenient place to override -
+ * it's used by the ialloc module, which is used by base64.
+ */
+# define _GL_INLINE_HEADER_BEGIN _Pragma ("GCC diagnostic push") \
+ void * \
+ reallocarray (void *ptr, unsigned int nmemb, unsigned int size);
+# define _GL_INLINE_HEADER_END _Pragma ("GCC diagnostic pop")
/*
* We don't have an abort() for gnulib to call in regexp. Because gnulib is
diff --git a/configure.ac b/configure.ac
index 5c01af0fa..6a8ae668d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,7 +49,7 @@ AC_CANONICAL_TARGET
program_prefix="${save_program_prefix}"
AM_INIT_AUTOMAKE([1.11])
-AC_PREREQ(2.63)
+AC_PREREQ(2.64)
AC_CONFIG_SRCDIR([include/grub/dl.h])
AC_CONFIG_HEADER([config-util.h])
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 8022e1c0a..a8f1d7c08 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -743,6 +743,9 @@ module = {
name = regexp;
common = commands/regexp.c;
common = commands/wildcard.c;
+ common = lib/gnulib/malloc/dynarray_finalize.c;
+ common = lib/gnulib/malloc/dynarray_emplace_enlarge.c;
+ common = lib/gnulib/malloc/dynarray_resize.c;
common = lib/gnulib/regex.c;
cflags = '$(CFLAGS_POSIX) $(CFLAGS_GNULIB)';
cppflags = '$(CPPFLAGS_POSIX) $(CPPFLAGS_GNULIB)';
diff --git a/grub-core/lib/gnulib-patches/fix-null-deref.patch b/grub-core/lib/gnulib-patches/fix-null-deref.patch
deleted file mode 100644
index 8fafa153a..000000000
--- a/grub-core/lib/gnulib-patches/fix-null-deref.patch
+++ /dev/null
@@ -1,13 +0,0 @@
-diff --git a/lib/argp-parse.c b/lib/argp-parse.c
-index 6dec57310..900adad54 100644
---- a/lib/argp-parse.c
-+++ b/lib/argp-parse.c
-@@ -940,7 +940,7 @@ weak_alias (__argp_parse, argp_parse)
- void *
- __argp_input (const struct argp *argp, const struct argp_state *state)
- {
-- if (state)
-+ if (state && state->pstate)
- {
- struct group *group;
- struct parser *parser = state->pstate;
diff --git a/grub-core/lib/gnulib-patches/fix-null-state-deref.patch b/grub-core/lib/gnulib-patches/fix-null-state-deref.patch
deleted file mode 100644
index 813ec09c8..000000000
--- a/grub-core/lib/gnulib-patches/fix-null-state-deref.patch
+++ /dev/null
@@ -1,12 +0,0 @@
---- a/lib/argp-help.c 2020-10-28 14:32:19.189215988 +0000
-+++ b/lib/argp-help.c 2020-10-28 14:38:21.204673940 +0000
-@@ -145,7 +145,8 @@
- if (*(int *)((char *)upptr + up->uparams_offs) >= upptr->rmargin)
- {
- __argp_failure (state, 0, 0,
-- dgettext (state->root_argp->argp_domain,
-+ dgettext (state == NULL ? NULL
-+ : state->root_argp->argp_domain,
- "\
- ARGP_HELP_FMT: %s value is less than or equal to %s"),
- "rmargin", up->name);
diff --git a/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch b/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
deleted file mode 100644
index 02e06315d..000000000
--- a/grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
+++ /dev/null
@@ -1,15 +0,0 @@
---- a/lib/regcomp.c 2020-11-24 17:06:08.159223858 +0000
-+++ b/lib/regcomp.c 2020-11-24 17:06:15.630253923 +0000
-@@ -3808,11 +3808,7 @@
- create_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right,
- re_token_type_t type)
- {
-- re_token_t t;
--#if defined GCC_LINT || defined lint
-- memset (&t, 0, sizeof t);
--#endif
-- t.type = type;
-+ re_token_t t = { .type = type };
- return create_token_tree (dfa, left, right, &t);
- }
-
diff --git a/grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch b/grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
deleted file mode 100644
index db6dac9c9..000000000
--- a/grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
+++ /dev/null
@@ -1,12 +0,0 @@
---- a/lib/regexec.c 2020-10-21 14:25:35.310195912 +0000
-+++ b/lib/regexec.c 2020-11-05 10:55:09.621542984 +0000
-@@ -1692,6 +1692,9 @@
- {
- Idx top = mctx->state_log_top;
-
-+ if (mctx->state_log == NULL)
-+ return REG_NOERROR;
-+
- if ((next_state_log_idx >= mctx->input.bufs_len
- && mctx->input.bufs_len < mctx->input.len)
- || (next_state_log_idx >= mctx->input.valid_len
diff --git a/grub-core/lib/gnulib-patches/fix-uninit-structure.patch b/grub-core/lib/gnulib-patches/fix-uninit-structure.patch
deleted file mode 100644
index 7b4d9f67a..000000000
--- a/grub-core/lib/gnulib-patches/fix-uninit-structure.patch
+++ /dev/null
@@ -1,11 +0,0 @@
---- a/lib/regcomp.c 2020-10-22 13:49:06.770168928 +0000
-+++ b/lib/regcomp.c 2020-10-22 13:50:37.026528298 +0000
-@@ -3662,7 +3662,7 @@
- Idx alloc = 0;
- #endif /* not RE_ENABLE_I18N */
- reg_errcode_t ret;
-- re_token_t br_token;
-+ re_token_t br_token = {0};
- bin_tree_t *tree;
-
- sbcset = (re_bitset_ptr_t) calloc (sizeof (bitset_t), 1);
diff --git a/grub-core/lib/gnulib-patches/fix-unused-value.patch b/grub-core/lib/gnulib-patches/fix-unused-value.patch
deleted file mode 100644
index ba51f1bf2..000000000
--- a/grub-core/lib/gnulib-patches/fix-unused-value.patch
+++ /dev/null
@@ -1,14 +0,0 @@
---- a/lib/regexec.c 2020-10-21 14:25:35.310195912 +0000
-+++ b/lib/regexec.c 2020-10-21 14:32:07.961765604 +0000
-@@ -828,7 +828,11 @@
- break;
- if (__glibc_unlikely (err != REG_NOMATCH))
- goto free_return;
-+#ifdef DEBUG
-+ /* Only used for assertion below when DEBUG is set, otherwise
-+ it will be over-written when we loop around. */
- match_last = -1;
-+#endif
- }
- else
- break; /* We found a match. */
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 6/6] Handle warnings introduced by updated gnulib
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
` (4 preceding siblings ...)
2022-03-02 19:08 ` [PATCH v8 5/6] Update gnulib version and drop most gnulib patches Robbie Harwood
@ 2022-03-02 19:08 ` Robbie Harwood
2022-03-04 18:16 ` [PATCH v8] Fix various new autotools warnings Robbie Harwood
2022-03-05 0:01 ` [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Glenn Washburn
7 siblings, 0 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-02 19:08 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
- Fix type of size variable in luks2_verify_key()
- Avoid redefinition of SIZE_MAX and ATTRIBUTE_ERROR
- Work around gnulib's int types on older compilers
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
config.h.in | 5 +++++
grub-core/disk/luks2.c | 4 ++--
grub-core/lib/posix_wrap/limits.h | 6 +++++-
include/grub/compiler.h | 4 ++--
include/grub/list.h | 2 +-
5 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/config.h.in b/config.h.in
index d0f4f8609..0fca0597d 100644
--- a/config.h.in
+++ b/config.h.in
@@ -145,4 +145,9 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
# define abort __builtin_trap
# endif /* !_GL_INLINE_HEADER_BEGIN */
+/* gnulib doesn't build cleanly with older compilers. */
+# if __GNUC__ < 11
+_Pragma ("GCC diagnostic ignored \"-Wtype-limits\"")
+# endif
+
#endif
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index ccfacb63a..bf741d70f 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -390,7 +390,7 @@ luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t *candidate_key,
{
grub_uint8_t candidate_digest[GRUB_CRYPTODISK_MAX_KEYLEN];
grub_uint8_t digest[GRUB_CRYPTODISK_MAX_KEYLEN], salt[GRUB_CRYPTODISK_MAX_KEYLEN];
- grub_size_t saltlen = sizeof (salt), digestlen = sizeof (digest);
+ idx_t saltlen = sizeof (salt), digestlen = sizeof (digest);
const gcry_md_spec_t *hash;
gcry_err_code_t gcry_ret;
@@ -429,7 +429,7 @@ luks2_decrypt_key (grub_uint8_t *out_key,
grub_uint8_t area_key[GRUB_CRYPTODISK_MAX_KEYLEN];
grub_uint8_t salt[GRUB_CRYPTODISK_MAX_KEYLEN];
grub_uint8_t *split_key = NULL;
- grub_size_t saltlen = sizeof (salt);
+ idx_t saltlen = sizeof (salt);
char cipher[32], *p;
const gcry_md_spec_t *hash;
gcry_err_code_t gcry_ret;
diff --git a/grub-core/lib/posix_wrap/limits.h b/grub-core/lib/posix_wrap/limits.h
index 7217138ff..26918c8a0 100644
--- a/grub-core/lib/posix_wrap/limits.h
+++ b/grub-core/lib/posix_wrap/limits.h
@@ -25,7 +25,11 @@
#define USHRT_MAX GRUB_USHRT_MAX
#define UINT_MAX GRUB_UINT_MAX
#define ULONG_MAX GRUB_ULONG_MAX
-#define SIZE_MAX GRUB_SIZE_MAX
+
+/* gnulib also defines this type */
+#ifndef SIZE_MAX
+# define SIZE_MAX GRUB_SIZE_MAX
+#endif
#define SCHAR_MIN GRUB_SCHAR_MIN
#define SCHAR_MAX GRUB_SCHAR_MAX
diff --git a/include/grub/compiler.h b/include/grub/compiler.h
index 8f3be3ae7..0c5519387 100644
--- a/include/grub/compiler.h
+++ b/include/grub/compiler.h
@@ -30,10 +30,10 @@
/* Does this compiler support compile-time error attributes? */
#if GNUC_PREREQ(4,3)
-# define ATTRIBUTE_ERROR(msg) \
+# define GRUB_ATTRIBUTE_ERROR(msg) \
__attribute__ ((__error__ (msg)))
#else
-# define ATTRIBUTE_ERROR(msg) __attribute__ ((noreturn))
+# define GRUB_ATTRIBUTE_ERROR(msg) __attribute__ ((noreturn))
#endif
#if GNUC_PREREQ(4,4)
diff --git a/include/grub/list.h b/include/grub/list.h
index b13acb962..21f4b4b44 100644
--- a/include/grub/list.h
+++ b/include/grub/list.h
@@ -40,7 +40,7 @@ void EXPORT_FUNC(grub_list_remove) (grub_list_t item);
static inline void *
grub_bad_type_cast_real (int line, const char *file)
- ATTRIBUTE_ERROR ("bad type cast between incompatible grub types");
+ GRUB_ATTRIBUTE_ERROR ("bad type cast between incompatible grub types");
static inline void *
grub_bad_type_cast_real (int line, const char *file)
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-02 19:08 ` [PATCH v8 4/6] Drop gnulib no-abort.patch Robbie Harwood
@ 2022-03-03 0:22 ` Glenn Washburn
2022-03-03 16:58 ` Robbie Harwood
0 siblings, 1 reply; 19+ messages in thread
From: Glenn Washburn @ 2022-03-03 0:22 UTC (permalink / raw)
To: Robbie Harwood; +Cc: The development of GNU GRUB, dkiper
On Wed, 2 Mar 2022 14:08:27 -0500
Robbie Harwood <rharwood@redhat.com> wrote:
> Originally added in db7337a3d353a817ffe9eb4a3702120527100be9, this
> patched out all relevant invocations of abort() in gnulib. While it was
> not documented why at the time, testing suggests that there's no abort()
> implementation available for gnulib to use.
>
> gnulib's position is that the use of abort() is correct here, since it
> happens when input violates a "shall" from POSIX. Additionally, the
> code in question is probably not reachable. Since abort() is more
> friendly to user-space, they prefer to make no change, so we can just
> carry a define instead. (Suggested by Paul Eggert.)
>
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
> bootstrap.conf | 2 +-
> conf/Makefile.extra-dist | 1 -
> config.h.in | 10 ++++++++
> grub-core/lib/gnulib-patches/no-abort.patch | 26 ---------------------
> 4 files changed, 11 insertions(+), 28 deletions(-)
> delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
>
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 21a8cf15d..71acbeeb1 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -82,7 +82,7 @@ cp -a INSTALL INSTALL.grub
> bootstrap_post_import_hook () {
> set -e
> for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
> - fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
> + fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width; do
> patch -d grub-core/lib/gnulib -p2 \
> < "grub-core/lib/gnulib-patches/$patchname.patch"
> done
> diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
> index 15a9b74e9..4ddc3c8f7 100644
> --- a/conf/Makefile.extra-dist
> +++ b/conf/Makefile.extra-dist
> @@ -35,7 +35,6 @@ EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
> -EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
>
> EXTRA_DIST += grub-core/lib/libgcrypt
> EXTRA_DIST += grub-core/lib/libgcrypt-grub/mpi/generic
> diff --git a/config.h.in b/config.h.in
> index 965eaffce..209e27449 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -66,6 +66,16 @@
>
> # ifndef _GL_INLINE_HEADER_BEGIN
> # define _GL_ATTRIBUTE_CONST __attribute__ ((const))
> +
> +/*
> + * We don't have an abort() for gnulib to call in regexp. Because gnulib is
> + * built as a separate object that is then linked with, it doesn't have any
> + * of our headers or functions around to use - so we unfortunately can't
> + * print anything. Builds of emu include the system's stdlib, which includes
> + * a prototype for abort(), so leave this as a macro that doesn't take
> + * arguments.
> + */
> +# define abort __builtin_trap
I asked earlier if we couldn't use grub_abort for gnulib's abort and
Daniel referred me to subsequent patch series. I suppose this is the
relevant section he was thinking of, however, its still not clear to me
why we can't use grub_abort(). And actually looking more into it, its
seems to me that using grub_abort() is probably what we should do
because it has platform specific cleanup code. It appears that
__builtin_trap is target dependent[1], but I do not believe that it is
or can be platform dependent.
Is the problem with grub_abort() that it provides some exit
guarantees[2] and we're looking for the equivalent of a machine crash?
That doesn't seem right to me. It is true that grub_abort calls
grub_exit which is platform specific. So for instance for x86_64-efi,
this will not call EFI boot_services->exit. Not being an EFI expert,
I'm not sure of the implications of that. If my suspicion is correct
and an abort in gnulib with this patch would not properly exit the EFI
app and not allow returning control to another EFI app, then I would
consider this patch undesirable. And I notice that other platforms have
special code run on grub_exit. I suspect that GCC's __builtin_trap is
more geared toward user-space programs and not for our use case.
If the problem is that gnulib, as stated above, is built as a separate
object and then linked to GRUB, which does not have an abort symbol
(thus causing a link failure). Then I think the right solution is to
create a symbol in the GRUB kernel named abort that has the same info as
grub_abort.
I have a patch I've been meaning to send which solves this problem for
GDB which in some cases look for symbols free() and malloc(). When
building the GRUB kernel add "-Wl,--defsym=abort=grub_abort" to the
LDFLAGS_KERNEL variable in conf/Makefile.common.
I haven't tested this, so I'm interested in hearing why this won't work
or isn't a good solution. I believe this should work for user-space
platforms like emu because of the platform specific grub_exit(). The one
problem I see is that for the emu platform exit guarantees may be
undesirable. This this case, perhaps grub_abort() should call libc's
abort().
Glenn
[1]
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005ftrap
[2] https://stackoverflow.com/a/35734843/2108011
> # endif /* !_GL_INLINE_HEADER_BEGIN */
>
> #endif
> diff --git a/grub-core/lib/gnulib-patches/no-abort.patch b/grub-core/lib/gnulib-patches/no-abort.patch
> deleted file mode 100644
> index e469c4762..000000000
> --- a/grub-core/lib/gnulib-patches/no-abort.patch
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -diff --git a/lib/regcomp.c b/lib/regcomp.c
> -index cc85f35ac..de45ebb5c 100644
> ---- a/lib/regcomp.c
> -+++ b/lib/regcomp.c
> -@@ -528,9 +528,9 @@ regerror (int errcode, const regex_t *__restrict preg, char *__restrict errbuf,
> - to this routine. If we are given anything else, or if other regex
> - code generates an invalid error code, then the program has a bug.
> - Dump core so we can fix it. */
> -- abort ();
> --
> -- msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
> -+ msg = gettext ("unknown regexp error");
> -+ else
> -+ msg = gettext (__re_error_msgid + __re_error_msgid_idx[errcode]);
> -
> - msg_size = strlen (msg) + 1; /* Includes the null. */
> -
> -@@ -1136,7 +1136,7 @@ optimize_utf8 (re_dfa_t *dfa)
> - }
> - break;
> - default:
> -- abort ();
> -+ break;
> - }
> -
> - if (mb_chars || has_period)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-03 0:22 ` Glenn Washburn
@ 2022-03-03 16:58 ` Robbie Harwood
2022-03-03 17:35 ` Glenn Washburn
0 siblings, 1 reply; 19+ messages in thread
From: Robbie Harwood @ 2022-03-03 16:58 UTC (permalink / raw)
To: development; +Cc: The development of GNU GRUB, dkiper
[-- Attachment #1: Type: text/plain, Size: 5668 bytes --]
Glenn Washburn <development@efficientek.com> writes:
> On Wed, 2 Mar 2022 14:08:27 -0500
> Robbie Harwood <rharwood@redhat.com> wrote:
>
>> Originally added in db7337a3d353a817ffe9eb4a3702120527100be9, this
>> patched out all relevant invocations of abort() in gnulib. While it was
>> not documented why at the time, testing suggests that there's no abort()
>> implementation available for gnulib to use.
>>
>> gnulib's position is that the use of abort() is correct here, since it
>> happens when input violates a "shall" from POSIX. Additionally, the
>> code in question is probably not reachable. Since abort() is more
>> friendly to user-space, they prefer to make no change, so we can just
>> carry a define instead. (Suggested by Paul Eggert.)
>>
>> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
>> ---
>> bootstrap.conf | 2 +-
>> conf/Makefile.extra-dist | 1 -
>> config.h.in | 10 ++++++++
>> grub-core/lib/gnulib-patches/no-abort.patch | 26 ---------------------
>> 4 files changed, 11 insertions(+), 28 deletions(-)
>> delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
>>
>> diff --git a/bootstrap.conf b/bootstrap.conf
>> index 21a8cf15d..71acbeeb1 100644
>> --- a/bootstrap.conf
>> +++ b/bootstrap.conf
>> @@ -82,7 +82,7 @@ cp -a INSTALL INSTALL.grub
>> bootstrap_post_import_hook () {
>> set -e
>> for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
>> - fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
>> + fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width; do
>> patch -d grub-core/lib/gnulib -p2 \
>> < "grub-core/lib/gnulib-patches/$patchname.patch"
>> done
>> diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
>> index 15a9b74e9..4ddc3c8f7 100644
>> --- a/conf/Makefile.extra-dist
>> +++ b/conf/Makefile.extra-dist
>> @@ -35,7 +35,6 @@ EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
>> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
>> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
>> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
>> -EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
>>
>> EXTRA_DIST += grub-core/lib/libgcrypt
>> EXTRA_DIST += grub-core/lib/libgcrypt-grub/mpi/generic
>> diff --git a/config.h.in b/config.h.in
>> index 965eaffce..209e27449 100644
>> --- a/config.h.in
>> +++ b/config.h.in
>> @@ -66,6 +66,16 @@
>>
>> # ifndef _GL_INLINE_HEADER_BEGIN
>> # define _GL_ATTRIBUTE_CONST __attribute__ ((const))
>> +
>> +/*
>> + * We don't have an abort() for gnulib to call in regexp. Because gnulib is
>> + * built as a separate object that is then linked with, it doesn't have any
>> + * of our headers or functions around to use - so we unfortunately can't
>> + * print anything. Builds of emu include the system's stdlib, which includes
>> + * a prototype for abort(), so leave this as a macro that doesn't take
>> + * arguments.
>> + */
>> +# define abort __builtin_trap
>
> I asked earlier if we couldn't use grub_abort for gnulib's abort and
> Daniel referred me to subsequent patch series. I suppose this is the
> relevant section he was thinking of, however, its still not clear to me
> why we can't use grub_abort(). And actually looking more into it, its
> seems to me that using grub_abort() is probably what we should do
> because it has platform specific cleanup code. It appears that
> __builtin_trap is target dependent[1], but I do not believe that it is
> or can be platform dependent.
>
> Is the problem with grub_abort() that it provides some exit
> guarantees[2] and we're looking for the equivalent of a machine crash?
> That doesn't seem right to me. It is true that grub_abort calls
> grub_exit which is platform specific. So for instance for x86_64-efi,
> this will not call EFI boot_services->exit. Not being an EFI expert,
> I'm not sure of the implications of that. If my suspicion is correct
> and an abort in gnulib with this patch would not properly exit the EFI
> app and not allow returning control to another EFI app, then I would
> consider this patch undesirable. And I notice that other platforms have
> special code run on grub_exit. I suspect that GCC's __builtin_trap is
> more geared toward user-space programs and not for our use case.
>
> If the problem is that gnulib, as stated above, is built as a separate
> object and then linked to GRUB, which does not have an abort symbol
> (thus causing a link failure). Then I think the right solution is to
> create a symbol in the GRUB kernel named abort that has the same info as
> grub_abort.
>
> I have a patch I've been meaning to send which solves this problem for
> GDB which in some cases look for symbols free() and malloc(). When
> building the GRUB kernel add "-Wl,--defsym=abort=grub_abort" to the
> LDFLAGS_KERNEL variable in conf/Makefile.common.
>
> I haven't tested this, so I'm interested in hearing why this won't work
> or isn't a good solution. I believe this should work for user-space
> platforms like emu because of the platform specific grub_exit(). The one
> problem I see is that for the emu platform exit guarantees may be
> undesirable. This this case, perhaps grub_abort() should call libc's
> abort().
If you have a patch that makes this work, I don't have a problem with
it. However, I was unable to make that work in practice.
Be well,
--Robbie
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-03 16:58 ` Robbie Harwood
@ 2022-03-03 17:35 ` Glenn Washburn
2022-03-03 18:47 ` Robbie Harwood
0 siblings, 1 reply; 19+ messages in thread
From: Glenn Washburn @ 2022-03-03 17:35 UTC (permalink / raw)
To: Robbie Harwood; +Cc: The development of GNU GRUB, dkiper
On Thu, 03 Mar 2022 11:58:11 -0500
Robbie Harwood <rharwood@redhat.com> wrote:
> Glenn Washburn <development@efficientek.com> writes:
>
> > On Wed, 2 Mar 2022 14:08:27 -0500
> > Robbie Harwood <rharwood@redhat.com> wrote:
> >
> >> Originally added in db7337a3d353a817ffe9eb4a3702120527100be9, this
> >> patched out all relevant invocations of abort() in gnulib. While it was
> >> not documented why at the time, testing suggests that there's no abort()
> >> implementation available for gnulib to use.
> >>
> >> gnulib's position is that the use of abort() is correct here, since it
> >> happens when input violates a "shall" from POSIX. Additionally, the
> >> code in question is probably not reachable. Since abort() is more
> >> friendly to user-space, they prefer to make no change, so we can just
> >> carry a define instead. (Suggested by Paul Eggert.)
> >>
> >> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> >> ---
> >> bootstrap.conf | 2 +-
> >> conf/Makefile.extra-dist | 1 -
> >> config.h.in | 10 ++++++++
> >> grub-core/lib/gnulib-patches/no-abort.patch | 26 ---------------------
> >> 4 files changed, 11 insertions(+), 28 deletions(-)
> >> delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
> >>
> >> diff --git a/bootstrap.conf b/bootstrap.conf
> >> index 21a8cf15d..71acbeeb1 100644
> >> --- a/bootstrap.conf
> >> +++ b/bootstrap.conf
> >> @@ -82,7 +82,7 @@ cp -a INSTALL INSTALL.grub
> >> bootstrap_post_import_hook () {
> >> set -e
> >> for patchname in fix-null-deref fix-null-state-deref fix-regcomp-uninit-token \
> >> - fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width no-abort; do
> >> + fix-regexec-null-deref fix-uninit-structure fix-unused-value fix-width; do
> >> patch -d grub-core/lib/gnulib -p2 \
> >> < "grub-core/lib/gnulib-patches/$patchname.patch"
> >> done
> >> diff --git a/conf/Makefile.extra-dist b/conf/Makefile.extra-dist
> >> index 15a9b74e9..4ddc3c8f7 100644
> >> --- a/conf/Makefile.extra-dist
> >> +++ b/conf/Makefile.extra-dist
> >> @@ -35,7 +35,6 @@ EXTRA_DIST += grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
> >> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-uninit-structure.patch
> >> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-unused-value.patch
> >> EXTRA_DIST += grub-core/lib/gnulib-patches/fix-width.patch
> >> -EXTRA_DIST += grub-core/lib/gnulib-patches/no-abort.patch
> >>
> >> EXTRA_DIST += grub-core/lib/libgcrypt
> >> EXTRA_DIST += grub-core/lib/libgcrypt-grub/mpi/generic
> >> diff --git a/config.h.in b/config.h.in
> >> index 965eaffce..209e27449 100644
> >> --- a/config.h.in
> >> +++ b/config.h.in
> >> @@ -66,6 +66,16 @@
> >>
> >> # ifndef _GL_INLINE_HEADER_BEGIN
> >> # define _GL_ATTRIBUTE_CONST __attribute__ ((const))
> >> +
> >> +/*
> >> + * We don't have an abort() for gnulib to call in regexp. Because gnulib is
> >> + * built as a separate object that is then linked with, it doesn't have any
> >> + * of our headers or functions around to use - so we unfortunately can't
> >> + * print anything. Builds of emu include the system's stdlib, which includes
> >> + * a prototype for abort(), so leave this as a macro that doesn't take
> >> + * arguments.
> >> + */
> >> +# define abort __builtin_trap
> >
> > I asked earlier if we couldn't use grub_abort for gnulib's abort and
> > Daniel referred me to subsequent patch series. I suppose this is the
> > relevant section he was thinking of, however, its still not clear to me
> > why we can't use grub_abort(). And actually looking more into it, its
> > seems to me that using grub_abort() is probably what we should do
> > because it has platform specific cleanup code. It appears that
> > __builtin_trap is target dependent[1], but I do not believe that it is
> > or can be platform dependent.
> >
> > Is the problem with grub_abort() that it provides some exit
> > guarantees[2] and we're looking for the equivalent of a machine crash?
> > That doesn't seem right to me. It is true that grub_abort calls
> > grub_exit which is platform specific. So for instance for x86_64-efi,
> > this will not call EFI boot_services->exit. Not being an EFI expert,
> > I'm not sure of the implications of that. If my suspicion is correct
> > and an abort in gnulib with this patch would not properly exit the EFI
> > app and not allow returning control to another EFI app, then I would
> > consider this patch undesirable. And I notice that other platforms have
> > special code run on grub_exit. I suspect that GCC's __builtin_trap is
> > more geared toward user-space programs and not for our use case.
> >
> > If the problem is that gnulib, as stated above, is built as a separate
> > object and then linked to GRUB, which does not have an abort symbol
> > (thus causing a link failure). Then I think the right solution is to
> > create a symbol in the GRUB kernel named abort that has the same info as
> > grub_abort.
> >
> > I have a patch I've been meaning to send which solves this problem for
> > GDB which in some cases look for symbols free() and malloc(). When
> > building the GRUB kernel add "-Wl,--defsym=abort=grub_abort" to the
> > LDFLAGS_KERNEL variable in conf/Makefile.common.
> >
> > I haven't tested this, so I'm interested in hearing why this won't work
> > or isn't a good solution. I believe this should work for user-space
> > platforms like emu because of the platform specific grub_exit(). The one
> > problem I see is that for the emu platform exit guarantees may be
> > undesirable. This this case, perhaps grub_abort() should call libc's
> > abort().
>
> If you have a patch that makes this work, I don't have a problem with
> it. However, I was unable to make that work in practice.
Can you provide some specifics on what problem you were running in to?
Was it a link issue at build time? platform specific? Did it build fine,
but blew up in testing? Did you try using the linker options I suggested
above?
I don't mind trying to get this to work, but I'm not sure if it'll be
soon.
I'm not convinced that the patch as is is actually the right thing to
do, and could actually be introducing a bug / undesirable behavior. I
suppose the impact is small because abort is an edge case. If we
ultimately do go with this, I think a FIXME or TODO comment should be
added saying that we should be using grub_abort() so that platform
specific cleanup code can be run.
Glenn
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-03 17:35 ` Glenn Washburn
@ 2022-03-03 18:47 ` Robbie Harwood
2022-03-04 17:40 ` Glenn Washburn
0 siblings, 1 reply; 19+ messages in thread
From: Robbie Harwood @ 2022-03-03 18:47 UTC (permalink / raw)
To: development; +Cc: The development of GNU GRUB, dkiper
[-- Attachment #1: Type: text/plain, Size: 4621 bytes --]
Glenn Washburn <development@efficientek.com> writes:
> Robbie Harwood <rharwood@redhat.com> wrote:
>
>> If you have a patch that makes this work, I don't have a problem with
>> it. However, I was unable to make that work in practice.
>
> Can you provide some specifics on what problem you were running in to?
> Was it a link issue at build time? platform specific? Did it build
> fine, but blew up in testing? Did you try using the linker options I
> suggested above?
No, nor am I about to. This is code that I have, that builds, that I'm
submitting to grub. If you want a different approach, that's fine -
you're welcome to write a patch to do that instead. I have built this
bike shed and I *really* do not care what color it is, nor do I
appreciate being asked to test other people's proposals for them: you're
presumably just as capable of building the code yourself and seeing if
something works or doesn't. This is v8 of the series and I'm pretty
much done caring about it at this point.
For completeness, here's what happens if one just defines to grub_abort
without further modification:
$ uname -m
x86_64
$ ./bootstrap
...
$ ./configure --enable-grub-mkfont
...
$ make
...
gcc -DHAVE_CONFIG_H -I. -I.. -Wall -W -DGRUB_MACHINE_PCBIOS=1 -DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/11/include -I../include -I../include -DGRUB_FILE=\"lib/gnulib/regex.c\" -I. -I. -I.. -I.. -I../include -I../include -I../grub-core/lib/libgcrypt-grub/src/ -I../grub-core/lib/posix_wrap -I../grub-core/lib/gnulib -I../grub-core/lib/gnulib -D_FILE_OFFSET_BITS=64 -std=gnu99 -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -mrtd -mregparm=3 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -Wa,-mx86-used-note=no -msoft-float -fno-dwarf2-cfi-asm -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -fno-PIE -fno-pie -fno-stack-protector -Wtrampolines -Werror -ffreestanding -fno-builtin -Wno-undef -Wno-sign-compare -Wno-unused -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code -Wno-conversion -MT lib/gnulib/regexp_module-regex.o -MD -MP -MF lib/gnulib/.deps-core/regexp_module-regex.Tpo -c -o lib/gnulib/regexp_module-regex.o `test -f 'lib/gnulib/regex.c' || echo './'`lib/gnulib/regex.c
In file included from ../grub-core/lib/gnulib/libc-config.h:36,
from lib/gnulib/regex.c:23:
lib/gnulib/regcomp.c: In function ‘regerror’:
../config.h:145:19: error: implicit declaration of function ‘grub_abort’; did you mean ‘grub_reboot’? [-Werror=implicit-function-declaration]
145 | # define abort grub_abort
| ^~~~~~~~~~
lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
509 | abort ();
| ^~~~~
../config.h:145:19: error: nested extern declaration of ‘grub_abort’ [-Werror=nested-externs]
145 | # define abort grub_abort
| ^~~~~~~~~~
lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
509 | abort ();
| ^~~~~
cc1: all warnings being treated as errors
...
$ git diff
diff --git a/config.h.in b/config.h.in
index 0fca0597d..8ad4aa0ac 100644
--- a/config.h.in
+++ b/config.h.in
@@ -142,7 +142,7 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
* a prototype for abort(), so leave this as a macro that doesn't take
* arguments.
*/
-# define abort __builtin_trap
+# define abort grub_abort
# endif /* !_GL_INLINE_HEADER_BEGIN */
/* gnulib doesn't build cleanly with older compilers. */
$
Be well,
--Robbie
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-03 18:47 ` Robbie Harwood
@ 2022-03-04 17:40 ` Glenn Washburn
2022-03-04 20:04 ` Daniel Kiper
0 siblings, 1 reply; 19+ messages in thread
From: Glenn Washburn @ 2022-03-04 17:40 UTC (permalink / raw)
To: Robbie Harwood; +Cc: The development of GNU GRUB, dkiper
On Thu, 03 Mar 2022 13:47:29 -0500
Robbie Harwood <rharwood@redhat.com> wrote:
> Glenn Washburn <development@efficientek.com> writes:
>
> > Robbie Harwood <rharwood@redhat.com> wrote:
> >
> >> If you have a patch that makes this work, I don't have a problem with
> >> it. However, I was unable to make that work in practice.
> >
> > Can you provide some specifics on what problem you were running in to?
> > Was it a link issue at build time? platform specific? Did it build
> > fine, but blew up in testing? Did you try using the linker options I
> > suggested above?
>
> No, nor am I about to. This is code that I have, that builds, that I'm
> submitting to grub. If you want a different approach, that's fine -
> you're welcome to write a patch to do that instead. I have built this
> bike shed and I *really* do not care what color it is, nor do I
> appreciate being asked to test other people's proposals for them: you're
> presumably just as capable of building the code yourself and seeing if
> something works or doesn't. This is v8 of the series and I'm pretty
> much done caring about it at this point.
Sounds like you're frustrated with the process and needed to let off
some stream. I can speak from experience on that subject (I got up to
v9 on a cryptodisk patch series that was actually fixing bugs). Of
course, its fine that you dont appreciate being asked to "test other
people's proposals" and its fine for anyone to ask you to do that,
which I did not (I asked if you had, there's a difference). You seem
capable of setting boundaries, as evidenced here, so no problem there.
There are many people on this list, including me, that have on occasion
modified a patch series according to someone else's proposal. I don't
think its an unreasonable ask, especially when the patch in question is
not correct. You're a free human being capable of saying no, as is your
perogative. There's no need to take it as a personal affront.
The GRUB project seems to me to have a fairly high code quality bar.
I've been frustrated in the past about what I considered bike shedding
as well and have made my frustrations known on and off this list. And I
think this is less trivial than some of those things.
Also, I think a more accurate metaphor, in this case, rather than
quibbling over color is whether you'll get electrocuted when lightening
strikes. This seems like its introducing a misfeature that someone else
down the line will get bitten by. So, if we can avoid that now when we
know there's an issue, the cost is much lower.
I hear you loud and clear saying you're done pushing on this patch
series. Would you like me to take it over? I've found what I believe to
be an acceptable solution using grub_abort.
>
> For completeness, here's what happens if one just defines to grub_abort
> without further modification:
>
> $ uname -m
> x86_64
> $ ./bootstrap
> ...
> $ ./configure --enable-grub-mkfont
> ...
> $ make
> ...
> gcc -DHAVE_CONFIG_H -I. -I.. -Wall -W -DGRUB_MACHINE_PCBIOS=1 -DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/11/include -I../include -I../include -DGRUB_FILE=\"lib/gnulib/regex.c\" -I. -I. -I.. -I.. -I../include -I../include -I../grub-core/lib/libgcrypt-grub/src/ -I../grub-core/lib/posix_wrap -I../grub-core/lib/gnulib -I../grub-core/lib/gnulib -D_FILE_OFFSET_BITS=64 -std=gnu99 -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -mrtd -mregparm=3 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -Wa,-mx86-used-note=no -msoft-float -fno-dwarf2-cfi-asm -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -fno-PIE -fno-pie -fno-stack-protector -Wtrampolines -Werror -ffreestanding -fno-builtin -Wno-undef -Wno-sign-compare -Wno-unused -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code -Wno-conversion -MT lib/gnulib/regexp_module-regex.o -MD -MP -MF lib/gnulib/.deps-core/regexp_module-regex.Tpo -c -o lib/gnulib/regexp_module-regex.o `test -f 'lib/gnulib/regex.c' || echo './'`lib/gnulib/regex.c
> In file included from ../grub-core/lib/gnulib/libc-config.h:36,
> from lib/gnulib/regex.c:23:
> lib/gnulib/regcomp.c: In function ‘regerror’:
> ../config.h:145:19: error: implicit declaration of function ‘grub_abort’; did you mean ‘grub_reboot’? [-Werror=implicit-function-declaration]
> 145 | # define abort grub_abort
> | ^~~~~~~~~~
> lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
> 509 | abort ();
> | ^~~~~
> ../config.h:145:19: error: nested extern declaration of ‘grub_abort’ [-Werror=nested-externs]
> 145 | # define abort grub_abort
> | ^~~~~~~~~~
> lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
> 509 | abort ();
> | ^~~~~
> cc1: all warnings being treated as errors
> ...
> $ git diff
> diff --git a/config.h.in b/config.h.in
> index 0fca0597d..8ad4aa0ac 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -142,7 +142,7 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
> * a prototype for abort(), so leave this as a macro that doesn't take
> * arguments.
> */
> -# define abort __builtin_trap
> +# define abort grub_abort
Thank you for letting me know what you tried and what was not working.
It turns out the issue was more complicated and different than I was let
on to believe based on the comment above. And while my link solution
could be part of the solution, its not necessary.
I'll send my changes to this thread shortly, unless you want me to roll
a v9. Thanks for getting this within a hare's breath of the finish line.
Glenn
> # endif /* !_GL_INLINE_HEADER_BEGIN */
>
> /* gnulib doesn't build cleanly with older compilers. */
> $
>
> Be well,
> --Robbie
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v8] Fix various new autotools warnings
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
` (5 preceding siblings ...)
2022-03-02 19:08 ` [PATCH v8 6/6] Handle warnings introduced by updated gnulib Robbie Harwood
@ 2022-03-04 18:16 ` Robbie Harwood
2022-03-05 0:01 ` [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Glenn Washburn
7 siblings, 0 replies; 19+ messages in thread
From: Robbie Harwood @ 2022-03-04 18:16 UTC (permalink / raw)
To: grub-devel; +Cc: dkiper, Robbie Harwood
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
configure.ac | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index 6a8ae668d..46b976562 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,7 @@ dnl description of the relationships between them.
AC_INIT([GRUB],[2.11],[bug-grub@gnu.org])
+AC_USE_SYSTEM_EXTENSIONS
AC_CONFIG_AUX_DIR([build-aux])
# We don't want -g -O2 by default in CFLAGS
@@ -51,7 +52,7 @@ program_prefix="${save_program_prefix}"
AM_INIT_AUTOMAKE([1.11])
AC_PREREQ(2.64)
AC_CONFIG_SRCDIR([include/grub/dl.h])
-AC_CONFIG_HEADER([config-util.h])
+AC_CONFIG_HEADERS([config-util.h])
# Explicitly check for pkg-config early on, since otherwise conditional
# calls are problematic.
@@ -333,7 +334,7 @@ fi
AC_PROG_RANLIB
AC_PROG_INSTALL
AC_PROG_AWK
-AC_PROG_LEX
+AC_PROG_LEX([noyywrap])
AC_PROG_YACC
AC_PROG_MAKE_SET
AC_PROG_MKDIR_P
@@ -369,7 +370,6 @@ test "x$GCC" = xyes || AC_MSG_ERROR([GCC is required])
AC_CHECK_PROG(HAVE_CXX, $CXX, yes, no)
-AC_GNU_SOURCE
AM_GNU_GETTEXT([external])
AM_GNU_GETTEXT_VERSION([0.18.3])
AC_SYS_LARGEFILE
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v8 4/6] Drop gnulib no-abort.patch
2022-03-04 17:40 ` Glenn Washburn
@ 2022-03-04 20:04 ` Daniel Kiper
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Kiper @ 2022-03-04 20:04 UTC (permalink / raw)
To: Glenn Washburn; +Cc: Robbie Harwood, The development of GNU GRUB
On Fri, Mar 04, 2022 at 11:40:10AM -0600, Glenn Washburn wrote:
> On Thu, 03 Mar 2022 13:47:29 -0500
> Robbie Harwood <rharwood@redhat.com> wrote:
> > Glenn Washburn <development@efficientek.com> writes:
> > > Robbie Harwood <rharwood@redhat.com> wrote:
[...]
> > For completeness, here's what happens if one just defines to grub_abort
> > without further modification:
> >
> > $ uname -m
> > x86_64
> > $ ./bootstrap
> > ...
> > $ ./configure --enable-grub-mkfont
> > ...
> > $ make
> > ...
> > gcc -DHAVE_CONFIG_H -I. -I.. -Wall -W -DGRUB_MACHINE_PCBIOS=1 -DGRUB_MACHINE=I386_PC -m32 -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/11/include -I../include -I../include -DGRUB_FILE=\"lib/gnulib/regex.c\" -I. -I. -I.. -I.. -I../include -I../include -I../grub-core/lib/libgcrypt-grub/src/ -I../grub-core/lib/posix_wrap -I../grub-core/lib/gnulib -I../grub-core/lib/gnulib -D_FILE_OFFSET_BITS=64 -std=gnu99 -Os -m32 -Wall -W -Wshadow -Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar -Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch -Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-parameter -Wunused-value -Wunused-variable -Wwrite-strings -Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes -Wmissing-declarations -Wextra -Wattributes -Wendif-labels -Winit-self -Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull -Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros -Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs -Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -mrtd -mregparm=3 -falign-functions=1 -falign-loops=1 -falign-jumps=1 -freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow -Wa,-mx86-used-note=no -msoft-float -fno-dwarf2-cfi-asm -mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-ident -fno-PIE -fno-pie -fno-stack-protector -Wtrampolines -Werror -ffreestanding -fno-builtin -Wno-undef -Wno-sign-compare -Wno-unused -Wno-unused-parameter -Wno-redundant-decls -Wno-unreachable-code -Wno-conversion -MT lib/gnulib/regexp_module-regex.o -MD -MP -MF lib/gnulib/.deps-core/regexp_module-regex.Tpo -c -o lib/gnulib/regexp_module-regex.o `test -f 'lib/gnulib/regex.c' || echo './'`lib/gnulib/regex.c
> > In file included from ../grub-core/lib/gnulib/libc-config.h:36,
> > from lib/gnulib/regex.c:23:
> > lib/gnulib/regcomp.c: In function ‘regerror’:
> > ../config.h:145:19: error: implicit declaration of function ‘grub_abort’; did you mean ‘grub_reboot’? [-Werror=implicit-function-declaration]
> > 145 | # define abort grub_abort
> > | ^~~~~~~~~~
> > lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
> > 509 | abort ();
> > | ^~~~~
> > ../config.h:145:19: error: nested extern declaration of ‘grub_abort’ [-Werror=nested-externs]
> > 145 | # define abort grub_abort
> > | ^~~~~~~~~~
> > lib/gnulib/regcomp.c:509:5: note: in expansion of macro ‘abort’
> > 509 | abort ();
> > | ^~~~~
> > cc1: all warnings being treated as errors
> > ...
> > $ git diff
> > diff --git a/config.h.in b/config.h.in
> > index 0fca0597d..8ad4aa0ac 100644
> > --- a/config.h.in
> > +++ b/config.h.in
> > @@ -142,7 +142,7 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
> > * a prototype for abort(), so leave this as a macro that doesn't take
> > * arguments.
> > */
> > -# define abort __builtin_trap
> > +# define abort grub_abort
>
> Thank you for letting me know what you tried and what was not working.
> It turns out the issue was more complicated and different than I was let
> on to believe based on the comment above. And while my link solution
> could be part of the solution, its not necessary.
>
> I'll send my changes to this thread shortly, unless you want me to roll
> a v9. Thanks for getting this within a hare's breath of the finish line.
Glenn, I am OK with improving this but I want to take v8 first to not
delay gnulib bump any longer. So, please post your patch(es) when Robbie
patches are in (Monday or Tuesday).
Daniel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/6] Update gnulib version and drop most gnulib patches
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
` (6 preceding siblings ...)
2022-03-04 18:16 ` [PATCH v8] Fix various new autotools warnings Robbie Harwood
@ 2022-03-05 0:01 ` Glenn Washburn
2022-03-07 8:15 ` Glenn Washburn
7 siblings, 1 reply; 19+ messages in thread
From: Glenn Washburn @ 2022-03-05 0:01 UTC (permalink / raw)
To: Robbie Harwood; +Cc: The development of GNU GRUB, dkiper
On Wed, 2 Mar 2022 14:08:23 -0500
Robbie Harwood <rharwood@redhat.com> wrote:
> Changes this version:
>
> - Reorder last two commits so that warning fixes come after the change that
> introduces them.
> - Fix comment formatting to comply with grub2 style.
Either I missed it before or something changed. But I'm getting this
build error now for x86_64-efi, and I'm not getting it without this patch
series.
In file included from /root/grub-tests.update-gnulib/grub/include/grub/disk.h:31
,
from /root/grub-tests.update-gnulib/grub/include/grub/file.h:26
,
from /root/grub-tests.update-gnulib/grub/include/grub/loader.h:
23,
from /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/
bsd.c:19:
/root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c: In function ‘gr
ub_freebsd_add_meta_module’:
/root/grub-tests.update-gnulib/grub/include/grub/misc.h:71:10: error: ‘ptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
71 | return grub_memmove (dest, src, n);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c:266:9: note: ‘ptr’ was declared here
266 | void *ptr;
| ^~~
Reviewing the code it doesn't look like ptr can actually be used
uninitialized, so it seems like GCC 10.1.0 isn't smart enough to figure
that out. Initializing to NULL fixes the build issue.
Glenn
>
> No functional code changes.
>
> Be well,
> --Robbie
>
> Robbie Harwood (6):
> Use visual indentation in config.h.in
> Where present, ensure config-util.h precedes config.h
> Drop gnulib fix-base64.patch
> Drop gnulib no-abort.patch
> Update gnulib version and drop most gnulib patches
> Handle warnings introduced by updated gnulib
>
> INSTALL | 4 +-
> bootstrap | 319 ++++++++++--------
> bootstrap.conf | 23 +-
> conf/Makefile.extra-dist | 8 -
> config.h.in | 142 ++++++--
> configure.ac | 2 +-
> grub-core/Makefile.core.def | 3 +
> grub-core/disk/host.c | 2 +-
> grub-core/disk/luks2.c | 4 +-
> grub-core/gensymlist.sh | 1 +
> grub-core/kern/emu/argp_common.c | 2 +-
> grub-core/kern/emu/main.c | 2 +-
> grub-core/lib/gnulib-patches/fix-base64.patch | 21 --
> .../lib/gnulib-patches/fix-null-deref.patch | 13 -
> .../gnulib-patches/fix-null-state-deref.patch | 12 -
> .../fix-regcomp-uninit-token.patch | 15 -
> .../fix-regexec-null-deref.patch | 12 -
> .../gnulib-patches/fix-uninit-structure.patch | 11 -
> .../lib/gnulib-patches/fix-unused-value.patch | 14 -
> grub-core/lib/gnulib-patches/no-abort.patch | 26 --
> grub-core/lib/posix_wrap/limits.h | 6 +-
> grub-core/lib/posix_wrap/sys/types.h | 7 +-
> grub-core/lib/xzembed/xz.h | 5 +-
> grub-core/osdep/aros/config.c | 2 +-
> grub-core/osdep/basic/emunet.c | 2 +-
> grub-core/osdep/basic/init.c | 2 +-
> grub-core/osdep/haiku/getroot.c | 2 +-
> grub-core/osdep/linux/emunet.c | 2 +-
> grub-core/osdep/unix/config.c | 2 +-
> grub-core/osdep/unix/cputime.c | 2 +-
> grub-core/osdep/unix/dl.c | 2 +-
> grub-core/osdep/unix/emuconsole.c | 2 +-
> grub-core/osdep/unix/getroot.c | 2 +-
> grub-core/osdep/windows/config.c | 2 +-
> grub-core/osdep/windows/cputime.c | 2 +-
> grub-core/osdep/windows/dl.c | 2 +-
> grub-core/osdep/windows/emuconsole.c | 2 +-
> grub-core/osdep/windows/init.c | 2 +-
> include/grub/compiler.h | 4 +-
> include/grub/list.h | 2 +-
> 40 files changed, 347 insertions(+), 343 deletions(-)
> delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
> delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch
> delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
> delete mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
> delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
> delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
> delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
> delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/6] Update gnulib version and drop most gnulib patches
2022-03-05 0:01 ` [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Glenn Washburn
@ 2022-03-07 8:15 ` Glenn Washburn
2022-03-09 15:42 ` Daniel Kiper
0 siblings, 1 reply; 19+ messages in thread
From: Glenn Washburn @ 2022-03-07 8:15 UTC (permalink / raw)
To: Robbie Harwood; +Cc: The development of GNU GRUB, dkiper
On Fri, 4 Mar 2022 18:01:10 -0600
Glenn Washburn <development@efficientek.com> wrote:
> On Wed, 2 Mar 2022 14:08:23 -0500
> Robbie Harwood <rharwood@redhat.com> wrote:
>
> > Changes this version:
> >
> > - Reorder last two commits so that warning fixes come after the change that
> > introduces them.
> > - Fix comment formatting to comply with grub2 style.
>
> Either I missed it before or something changed. But I'm getting this
> build error now for x86_64-efi, and I'm not getting it without this patch
> series.
>
> In file included from /root/grub-tests.update-gnulib/grub/include/grub/disk.h:31
> ,
> from /root/grub-tests.update-gnulib/grub/include/grub/file.h:26
> ,
> from /root/grub-tests.update-gnulib/grub/include/grub/loader.h:
> 23,
> from /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/
> bsd.c:19:
> /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c: In function ‘gr
> ub_freebsd_add_meta_module’:
> /root/grub-tests.update-gnulib/grub/include/grub/misc.h:71:10: error: ‘ptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 71 | return grub_memmove (dest, src, n);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c:266:9: note: ‘ptr’ was declared here
> 266 | void *ptr;
> | ^~~
>
> Reviewing the code it doesn't look like ptr can actually be used
> uninitialized, so it seems like GCC 10.1.0 isn't smart enough to figure
> that out. Initializing to NULL fixes the build issue.
For completeness, I just realized that I wasn't using the compiler I
thought I was using. Turns out this issue is with x86_64 Debian 11's
GCC, which is 10.2.1. I don't think this is relevant to the issue, but
may help in reproducing if needed.
After some more testing, I've narrowed it down to passing -O2 to gcc,
without which the series builds fine.
Glenn
>
> >
> > No functional code changes.
> >
> > Be well,
> > --Robbie
> >
> > Robbie Harwood (6):
> > Use visual indentation in config.h.in
> > Where present, ensure config-util.h precedes config.h
> > Drop gnulib fix-base64.patch
> > Drop gnulib no-abort.patch
> > Update gnulib version and drop most gnulib patches
> > Handle warnings introduced by updated gnulib
> >
> > INSTALL | 4 +-
> > bootstrap | 319 ++++++++++--------
> > bootstrap.conf | 23 +-
> > conf/Makefile.extra-dist | 8 -
> > config.h.in | 142 ++++++--
> > configure.ac | 2 +-
> > grub-core/Makefile.core.def | 3 +
> > grub-core/disk/host.c | 2 +-
> > grub-core/disk/luks2.c | 4 +-
> > grub-core/gensymlist.sh | 1 +
> > grub-core/kern/emu/argp_common.c | 2 +-
> > grub-core/kern/emu/main.c | 2 +-
> > grub-core/lib/gnulib-patches/fix-base64.patch | 21 --
> > .../lib/gnulib-patches/fix-null-deref.patch | 13 -
> > .../gnulib-patches/fix-null-state-deref.patch | 12 -
> > .../fix-regcomp-uninit-token.patch | 15 -
> > .../fix-regexec-null-deref.patch | 12 -
> > .../gnulib-patches/fix-uninit-structure.patch | 11 -
> > .../lib/gnulib-patches/fix-unused-value.patch | 14 -
> > grub-core/lib/gnulib-patches/no-abort.patch | 26 --
> > grub-core/lib/posix_wrap/limits.h | 6 +-
> > grub-core/lib/posix_wrap/sys/types.h | 7 +-
> > grub-core/lib/xzembed/xz.h | 5 +-
> > grub-core/osdep/aros/config.c | 2 +-
> > grub-core/osdep/basic/emunet.c | 2 +-
> > grub-core/osdep/basic/init.c | 2 +-
> > grub-core/osdep/haiku/getroot.c | 2 +-
> > grub-core/osdep/linux/emunet.c | 2 +-
> > grub-core/osdep/unix/config.c | 2 +-
> > grub-core/osdep/unix/cputime.c | 2 +-
> > grub-core/osdep/unix/dl.c | 2 +-
> > grub-core/osdep/unix/emuconsole.c | 2 +-
> > grub-core/osdep/unix/getroot.c | 2 +-
> > grub-core/osdep/windows/config.c | 2 +-
> > grub-core/osdep/windows/cputime.c | 2 +-
> > grub-core/osdep/windows/dl.c | 2 +-
> > grub-core/osdep/windows/emuconsole.c | 2 +-
> > grub-core/osdep/windows/init.c | 2 +-
> > include/grub/compiler.h | 4 +-
> > include/grub/list.h | 2 +-
> > 40 files changed, 347 insertions(+), 343 deletions(-)
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-base64.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-null-deref.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-null-state-deref.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-regcomp-uninit-token.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-regexec-null-deref.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-uninit-structure.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/fix-unused-value.patch
> > delete mode 100644 grub-core/lib/gnulib-patches/no-abort.patch
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/6] Update gnulib version and drop most gnulib patches
2022-03-07 8:15 ` Glenn Washburn
@ 2022-03-09 15:42 ` Daniel Kiper
2022-03-09 20:01 ` Glenn Washburn
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Kiper @ 2022-03-09 15:42 UTC (permalink / raw)
To: Glenn Washburn; +Cc: Robbie Harwood, The development of GNU GRUB
On Mon, Mar 07, 2022 at 02:15:49AM -0600, Glenn Washburn wrote:
> On Fri, 4 Mar 2022 18:01:10 -0600
> Glenn Washburn <development@efficientek.com> wrote:
>
> > On Wed, 2 Mar 2022 14:08:23 -0500
> > Robbie Harwood <rharwood@redhat.com> wrote:
> >
> > > Changes this version:
> > >
> > > - Reorder last two commits so that warning fixes come after the change that
> > > introduces them.
> > > - Fix comment formatting to comply with grub2 style.
> >
> > Either I missed it before or something changed. But I'm getting this
> > build error now for x86_64-efi, and I'm not getting it without this patch
> > series.
> >
> > In file included from /root/grub-tests.update-gnulib/grub/include/grub/disk.h:31
> > ,
> > from /root/grub-tests.update-gnulib/grub/include/grub/file.h:26
> > ,
> > from /root/grub-tests.update-gnulib/grub/include/grub/loader.h:
> > 23,
> > from /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/
> > bsd.c:19:
> > /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c: In function ‘gr
> > ub_freebsd_add_meta_module’:
> > /root/grub-tests.update-gnulib/grub/include/grub/misc.h:71:10: error: ‘ptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 71 | return grub_memmove (dest, src, n);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c:266:9: note: ‘ptr’ was declared here
> > 266 | void *ptr;
> > | ^~~
> >
> > Reviewing the code it doesn't look like ptr can actually be used
> > uninitialized, so it seems like GCC 10.1.0 isn't smart enough to figure
> > that out. Initializing to NULL fixes the build issue.
>
> For completeness, I just realized that I wasn't using the compiler I
> thought I was using. Turns out this issue is with x86_64 Debian 11's
> GCC, which is 10.2.1. I don't think this is relevant to the issue, but
> may help in reproducing if needed.
>
> After some more testing, I've narrowed it down to passing -O2 to gcc,
> without which the series builds fine.
Thanks for the report. I am just running build tests for all architectures
and platforms with GCC 10/11. I have spotted and fixed 2 additional problems.
I hope I will post fixes for all of them tomorrow.
Robbie and I discovered another gnulib build issue for Windows platforms.
I prepared a patch and will share it with Robbie to send it in updated
gnulib v9 patchset.
Daniel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/6] Update gnulib version and drop most gnulib patches
2022-03-09 15:42 ` Daniel Kiper
@ 2022-03-09 20:01 ` Glenn Washburn
2022-03-10 23:44 ` Daniel Kiper
0 siblings, 1 reply; 19+ messages in thread
From: Glenn Washburn @ 2022-03-09 20:01 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Robbie Harwood, The development of GNU GRUB
On Wed, 9 Mar 2022 16:42:43 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:
> On Mon, Mar 07, 2022 at 02:15:49AM -0600, Glenn Washburn wrote:
> > On Fri, 4 Mar 2022 18:01:10 -0600
> > Glenn Washburn <development@efficientek.com> wrote:
> >
> > > On Wed, 2 Mar 2022 14:08:23 -0500
> > > Robbie Harwood <rharwood@redhat.com> wrote:
> > >
> > > > Changes this version:
> > > >
> > > > - Reorder last two commits so that warning fixes come after the change that
> > > > introduces them.
> > > > - Fix comment formatting to comply with grub2 style.
> > >
> > > Either I missed it before or something changed. But I'm getting this
> > > build error now for x86_64-efi, and I'm not getting it without this patch
> > > series.
> > >
> > > In file included from /root/grub-tests.update-gnulib/grub/include/grub/disk.h:31
> > > ,
> > > from /root/grub-tests.update-gnulib/grub/include/grub/file.h:26
> > > ,
> > > from /root/grub-tests.update-gnulib/grub/include/grub/loader.h:
> > > 23,
> > > from /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/
> > > bsd.c:19:
> > > /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c: In function ‘gr
> > > ub_freebsd_add_meta_module’:
> > > /root/grub-tests.update-gnulib/grub/include/grub/misc.h:71:10: error: ‘ptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > 71 | return grub_memmove (dest, src, n);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c:266:9: note: ‘ptr’ was declared here
> > > 266 | void *ptr;
> > > | ^~~
> > >
> > > Reviewing the code it doesn't look like ptr can actually be used
> > > uninitialized, so it seems like GCC 10.1.0 isn't smart enough to figure
> > > that out. Initializing to NULL fixes the build issue.
> >
> > For completeness, I just realized that I wasn't using the compiler I
> > thought I was using. Turns out this issue is with x86_64 Debian 11's
> > GCC, which is 10.2.1. I don't think this is relevant to the issue, but
> > may help in reproducing if needed.
> >
> > After some more testing, I've narrowed it down to passing -O2 to gcc,
> > without which the series builds fine.
>
> Thanks for the report. I am just running build tests for all architectures
> and platforms with GCC 10/11. I have spotted and fixed 2 additional problems.
> I hope I will post fixes for all of them tomorrow.
They are not, build errors, are they? If so, I'm curious what targets
they are for and why I'm not seeing them either.
Glenn
>
> Robbie and I discovered another gnulib build issue for Windows platforms.
> I prepared a patch and will share it with Robbie to send it in updated
> gnulib v9 patchset.
>
> Daniel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/6] Update gnulib version and drop most gnulib patches
2022-03-09 20:01 ` Glenn Washburn
@ 2022-03-10 23:44 ` Daniel Kiper
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Kiper @ 2022-03-10 23:44 UTC (permalink / raw)
To: Glenn Washburn; +Cc: Robbie Harwood, The development of GNU GRUB
On Wed, Mar 09, 2022 at 02:01:16PM -0600, Glenn Washburn wrote:
> On Wed, 9 Mar 2022 16:42:43 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Mon, Mar 07, 2022 at 02:15:49AM -0600, Glenn Washburn wrote:
> > > On Fri, 4 Mar 2022 18:01:10 -0600
> > > Glenn Washburn <development@efficientek.com> wrote:
> > >
> > > > On Wed, 2 Mar 2022 14:08:23 -0500
> > > > Robbie Harwood <rharwood@redhat.com> wrote:
> > > >
> > > > > Changes this version:
> > > > >
> > > > > - Reorder last two commits so that warning fixes come after the change that
> > > > > introduces them.
> > > > > - Fix comment formatting to comply with grub2 style.
> > > >
> > > > Either I missed it before or something changed. But I'm getting this
> > > > build error now for x86_64-efi, and I'm not getting it without this patch
> > > > series.
> > > >
> > > > In file included from /root/grub-tests.update-gnulib/grub/include/grub/disk.h:31
> > > > ,
> > > > from /root/grub-tests.update-gnulib/grub/include/grub/file.h:26
> > > > ,
> > > > from /root/grub-tests.update-gnulib/grub/include/grub/loader.h:
> > > > 23,
> > > > from /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/
> > > > bsd.c:19:
> > > > /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c: In function ‘gr
> > > > ub_freebsd_add_meta_module’:
> > > > /root/grub-tests.update-gnulib/grub/include/grub/misc.h:71:10: error: ‘ptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > 71 | return grub_memmove (dest, src, n);
> > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /root/grub-tests.update-gnulib/grub/grub-core/loader/i386/bsd.c:266:9: note: ‘ptr’ was declared here
> > > > 266 | void *ptr;
> > > > | ^~~
> > > >
> > > > Reviewing the code it doesn't look like ptr can actually be used
> > > > uninitialized, so it seems like GCC 10.1.0 isn't smart enough to figure
> > > > that out. Initializing to NULL fixes the build issue.
> > >
> > > For completeness, I just realized that I wasn't using the compiler I
> > > thought I was using. Turns out this issue is with x86_64 Debian 11's
> > > GCC, which is 10.2.1. I don't think this is relevant to the issue, but
> > > may help in reproducing if needed.
> > >
> > > After some more testing, I've narrowed it down to passing -O2 to gcc,
> > > without which the series builds fine.
> >
> > Thanks for the report. I am just running build tests for all architectures
> > and platforms with GCC 10/11. I have spotted and fixed 2 additional problems.
> > I hope I will post fixes for all of them tomorrow.
>
> They are not, build errors, are they? If so, I'm curious what targets
Yep, they are. I have sent fixes. You are CC-ed.
Daniel
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-03-10 23:44 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-02 19:08 [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 1/6] Use visual indentation in config.h.in Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 2/6] Where present, ensure config-util.h precedes config.h Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 3/6] Drop gnulib fix-base64.patch Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 4/6] Drop gnulib no-abort.patch Robbie Harwood
2022-03-03 0:22 ` Glenn Washburn
2022-03-03 16:58 ` Robbie Harwood
2022-03-03 17:35 ` Glenn Washburn
2022-03-03 18:47 ` Robbie Harwood
2022-03-04 17:40 ` Glenn Washburn
2022-03-04 20:04 ` Daniel Kiper
2022-03-02 19:08 ` [PATCH v8 5/6] Update gnulib version and drop most gnulib patches Robbie Harwood
2022-03-02 19:08 ` [PATCH v8 6/6] Handle warnings introduced by updated gnulib Robbie Harwood
2022-03-04 18:16 ` [PATCH v8] Fix various new autotools warnings Robbie Harwood
2022-03-05 0:01 ` [PATCH v8 0/6] Update gnulib version and drop most gnulib patches Glenn Washburn
2022-03-07 8:15 ` Glenn Washburn
2022-03-09 15:42 ` Daniel Kiper
2022-03-09 20:01 ` Glenn Washburn
2022-03-10 23:44 ` Daniel Kiper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.