grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 00/13] error: Do compile-time format string checking on gr


From: Daniel Kiper
Subject: Re: [PATCH v4 00/13] error: Do compile-time format string checking on grub_error
Date: Sat, 27 Feb 2021 13:19:17 +0100
User-agent: NeoMutt/20170113 (1.7.2)

On Thu, Feb 18, 2021 at 08:47:01PM -0600, Glenn Washburn wrote:
> This patch series fixes all compile errors due to format string issues on
> grub_error. This was tested against nearly all supported platforms
> successfully. This is important because earlier versions of these changes
> compiled successfully on x86 platforms, but had issues on other ones not
> tested.

Could you give examples what kind of errors you get when you build
non-x86 platforms?

> All patches except the last fix actual format string issues. The last patch
> turns format string issues into errors. This is a good idea because it will
> help to prevent introducing new format string issues into the code. Since, I
> presume, Daniel does not do not do a build test for all architectures before
> committing to master, this will not ensure that no format string issues get
> introduced into the code. However, it should flush out any format string

I am confused by these sentence. Anyway, I do build tests of all patches
before committing for all architectures which I am aware of. I think
difference between our results come from difference in build
environments, flags and options to do tests.

> issues when the CI build is done.
>
> Many of these changes are fairly obvious. I tried to use the PRI*GRUB_*_T
> macros as much as I could and to not cast arguments. There are some notable
> exceptions. There is some code that uses the same grub_error code in both
> 32 and 64 bit architectures (riscv), so casting was needed to force the larger
> storage type. The second to last patch for zfs is still confounding to me
> as to why the macro PRIuGRUB_UINT64_T was not expanding to llu, and yet the
> compiler was saying the argument was a long long unsigned.

For all the patches except #12 Reviewed-by: Daniel Kiper 
<daniel.kiper@oracle.com>

Though I will probably do not get #13 until #12 is updated and merged.

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]