On Sat, 20 Apr 2024 at 17:31, Michael Brown wrote:
>
> On 19/04/2024 11:02, Mike Beaton wrote:
> > Dear Michael,
> >
> > I don't know if you had time to answer one follow-up question.
> >
> > Obviously one thing that someone might want to do is to notify on
>
not saying you're wrong, just that it would be helpful (to me!) to
understand what sort of thing you were thinking of!
Many thanks in advance for any time you might have to reply.
Mike Beaton
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online
Does anybody have any suggestions who I should cc as regards this?
Is it preferred to make a bugtracker issue too?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117500): https://edk2.groups.io/g/devel/message/117500
Mute This Topic:
: Michael Brown
Cc: Maciej Rabeda
Cc: Jiaxin Wu
Signed-off-by: Mike Beaton
xyz
---
NetworkPkg/HttpBootDxe/HttpBootImpl.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index
confirms that the modified uninstall is succeeding.
Cc: Maciej Rabeda
Cc: Jiaxin Wu
Cc: Siyuan Fu
Signed-off-by: Mike Beaton
---
NetworkPkg/HttpBootDxe/HttpBootImpl.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/NetworkPkg/HttpBootDxe
> For the exception handler case, we can just drop the #Ifdefs around
> the definition of BaseName () entirely given that it will now always
> be referenced. But that does depend a lot on how other toolchains deal
> with this (VS201x primarily)
The V5/V6 version of the patch builds on VS2019
I did ask. Thank you for the considered answer. Ack. :)
On Thu, 14 Dec 2023 at 13:25, Laszlo Ersek wrote:
>
> On 12/14/23 10:33, Mike Beaton wrote:
> >> Please stop sending patches.
> >
> > I believe this version is a clear improvement, with motivation.
&
> IOW, please don't send a v6 until the discussion comes to a conclusion.
Apologies, I did _not_ see this before sending.
> > - #if !defined (MDEPKG_NDEBUG)
> > + #if defined (__CC_ARM) || defined (__GNUC__)
>
> No, this is not going to be acceptable to me. You noted that only the
> ARM code
> Please stop sending patches.
I believe this version is a clear improvement, with motivation.
(Certainly, it was meant as such!)
How am I meant to send improvements or updates in this email-based workflow?
Mike
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this
Repeats V5, but with a hopefully clearer (on motivation and history) commit
message.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112511): https://edk2.groups.io/g/devel/message/112511
Mute This Topic:
From: Mike Beaton
The variant provided when MDEPKG_NDEBUG is defined will be optimised
away in RELEASE builds, but by referencing the argument list, avoids
unused variable errors from valid debug code, for example when STATIC
variables are used only in DEBUG statements.
This issue was being
From: Mike Beaton
The variant provided when MDEPKG_NDEBUG is defined will be optimised
away in RELEASE builds, but by referencing the argument list, avoids
unused variable errors from valid debug code, for example when STATIC
variables are used only in DEBUG statements.
Variable EventNames
Hi, I'm not fully used to email-based git commits. In this case, I've got two
commits with different messages which are meant to make one patch set. I was
under the impression that the first line of each commit is taken from the
subject, but in that case these cease to appear as one thread in
Apologies - sent in error without V4 tag, please ignore.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112502): https://edk2.groups.io/g/devel/message/112502
Mute This Topic: https://groups.io/mt/103166248/21656
Group Owner:
From: Mike Beaton
This is no longer required since the revised DEBUG macro automatically
compiles away unused var accesses when MDEPKG_NDEBUG is defined;
keeping these lines is incompatible with the updated DEBUG macro, as
there has to be a variable, access to which to discard.
Signed-off
From: Mike Beaton
The variant provided when MDEPKG_NDEBUG is defined will be optimised
away in RELEASE builds, but by referencing the argument list, avoids
unused variable errors from valid debug code, for example when STATIC
variables are used only in DEBUG statements.
Variables EventNames
From: Mike Beaton
The variant provided when MDEPKG_NDEBUG is defined will be optimised
away in RELEASE builds, but by referencing the argument list, avoids
unused variable errors from valid debug code, for example when STATIC
variables are used only in DEBUG statements.
Variables EventNames
Got it. These are the fixes for ArmVirtQemu.dsc. If people have
already manually worked round this issue it interferes with this
global fix (we had this in one file in Acidanthera code). There don't
seem to be many instances of that in the entire EDK-II codebase,
however, fortunately.
diff --git
I have currently gone with `if (FALSE)` in the above, even though
Michael Brown kindly offered tests which showed that `if ((FALSE))`
might help in a closely related context - i.e. when discussing
Mikhail's variant of this and related code - two years ago now:
From: Mike Beaton
The variant provided when MDEPKG_NDEBUG is defined will be optimised
away in RELEASE builds, but by referencing the argument list, avoids
unused variable errors from valid debug code, for example when STATIC
variables are used only in DEBUG statements.
Signed-off-by: Mike
> > +#define DEBUG(Expression)\
> > +do { \
> > + _Pragma("GCC diagnostic push") \
> > + _Pragma("GCC diagnostic ignored \"-Wunused-value\"") \
> > + if ((FALSE)) {
sed variables elsewhere.
>
> Note that the _Pragma in the clang/gcc variant is to temporarily
> suppress the warning about `(VOID) Expression;`, whilst the presence
> of `(VOID) Expression;` (once allowed) is what prevents the unwanted
> warning about unneeded variables.
>
> Signed-o
the warning about `(VOID) Expression;`, whilst the presence
of `(VOID) Expression;` (once allowed) is what prevents the unwanted
warning about unneeded variables.
Signed-off-by: Mikhail Krichanov
Signed-off-by: Mike Beaton
---
BaseTools/Conf/tools_def.template | 2 +-
MdePkg/Include/Library
> I believe one way to approach it would be to (a) amend the patch with
>
>--author="Marvin Häuser "
>
> and (b) have S-o-b's from both Marvin and you at the end of the commit
> message.
>
As I say, I think the code in question is actual by Mikhail, despite
appearances, but I should be able
> You failed to mention where you found this patch.
It's from https://github.com/acidanthera/audk/commit/dcd0a768b0f
(which actually contains the code described in its commit message, but
I believe some other code, including this specific part, which got
squashed in at some point, and I believe
> This seems redundant to me. Either we set the pragma and the compiler
> does not care, or we don't, and rely on the fact that the compiler can
> infer that 'Expression' will never be evaluated at runtime, but won't
> complain about symbols that are only referenced via 'Expression' and
> nowhere
> I'm running some further tests to confirm that the simple version
> proposed above builds in all the Acidanthera test environments, which
> is relevant in that it should rule out at least any obvious problems
> with it building in all edk-2 environments, too. (I had already run
> quick tests for
> I have realised that this was already fixed (i.e. allowing keeping the
> warning) in Acidanthera fork of EDK-II. Discussed here
> https://bugzilla.tianocore.org/show_bug.cgi?id=3704 - includes the fix
> in question and other fixes for newer gcc as well. I'll post a new
> patch to the list
From: Mike Beaton
Provides a variant of the DEBUG macro for clang when MDEPKG_NDEBUG is defined,
which uses but discards the contained expression, this means clang can tell
that it has optimised away variable usage, therefore we can keep
-Wunneeded-internal-declaration (as part of -Wall) to warn
the relevant fix for clang.
On Tue, 12 Dec 2023 at 07:49, Ard Biesheuvel wrote:
>
> On Tue, 12 Dec 2023 at 08:17, Mike Beaton wrote:
> >
> > > > A completely different approach, which allows clang to spot that the
> > > > usage has been 'optimised away' a
> > A completely different approach, which allows clang to spot that the
> > usage has been 'optimised away' and so to not complain (and therefore
> > allows us to re-enable the warning in CLANGDWARF as well), is the
> > following:
> >
> > --- a/MdePkg/Include/Library/DebugLib.h
> > +++
> > I believe this would be logically wrong, as the other versions still
> > wouldn't compile if you changed the relevant debug Pcds. (Which are
> > logically independent of the compile and link options - e.g. what if for
> > some reason you wanted to single step with the Debug Pcds set to
> >
build?)
On Mon, 11 Dec 2023, 15:00 Laszlo Ersek, wrote:
> On 12/10/23 11:18, Mike Beaton wrote:
> > From: Mike Beaton
> >
> > This warning was already disabled in CLANGDWARF by commit
> > d3225577123767fd09c91201d27e9c91663ae132.
> >
> > gcc can distinguis
Resolves https://bugzilla.tianocore.org/show_bug.cgi?id=4620
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112262): https://edk2.groups.io/g/devel/message/112262
Mute This Topic: https://groups.io/mt/103087794/21656
Group Owner:
Repeats the commit already acked by Ard in
https://edk2.groups.io/g/devel/topic/103083030, though with an attempt
to provide an additional (not yet acked) useful commit message.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112261):
I've sent a patch to this list (with some kind of proposed comment
about the issues!) under separate cover.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112260): https://edk2.groups.io/g/devel/message/112260
Mute This Topic:
From: Mike Beaton
This warning was already disabled in CLANGDWARF by commit
d3225577123767fd09c91201d27e9c91663ae132.
gcc can distinguish between optimised-away variable usage (as can occur in
valid debug code) and genuinely unused variables, and only complains about
the latter. clang cannot
> Removing STATIC means that (modulo LTO) the compiler will not know
> whether or not the definition can be dropped. It also pollutes the
> global namespace.
>
> IMO, lack of the use of STATIC where appropriate is a severe issue
> with the EDK2 codebase. Removing STATIC to appease compiler
>
Dear Ard,
Thanks for your attention on my other issue, about NOOPT CLANGDWARF OVMF.
This one is about RELEASE CLANGPDB OVMF, which currently does not
compile, giving:
```
/home/mjsbeaton/OpenSource/edk2/OvmfPkg/VirtioSerialDxe/VirtioSerial.c:28:22:
error: variable 'EventNames' is not needed and
removed) pragma?
On Sat, 9 Dec 2023 at 15:24, Mike Beaton wrote:
>
> I have raised a bug,
> https://bugzilla.tianocore.org/show_bug.cgi?id=4617 - adding a little
> bit of new information which I believe is correct, that I've been able
> to dig out.
-=-=-=-=-=-=-=-=-=-=-=-
Group
I have raised a bug,
https://bugzilla.tianocore.org/show_bug.cgi?id=4617 - adding a little
bit of new information which I believe is correct, that I've been able
to dig out.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112251):
> - I have just replicated exactly the same issue from a clean, new
> checkout and build on Fedora 38 and clang 16 (Fedora 16.0.6-3.fc38).
> - I then upgraded that same VM to Fedora 39 and clang 17 and ... I
> still get the same issue, including that it works again when switching
> to
> > Which clang version is this?
> >
> > It works for me, fedora 39 / clang 17, and I'm pretty sure I tested it
> > when creating the patch on a slightly older clang version (15 or 16).
>
> It is 14 ('Ubuntu clang version 14.0.0-1ubuntu1.1' on Ubuntu 22.04.3 LTS)
>
> Cheers,
> Mike
Strangely,
> Which clang version is this?
>
> It works for me, fedora 39 / clang 17, and I'm pretty sure I tested it
> when creating the patch on a slightly older clang version (15 or 16).
It is 14 ('Ubuntu clang version 14.0.0-1ubuntu1.1' on Ubuntu 22.04.3 LTS)
Cheers,
Mike
-=-=-=-=-=-=-=-=-=-=-=-
>From commit
>https://github.com/tianocore/edk2/commit/140e4422b16482f0bcafdc20d42141434d450450
(and see also the preceding commit, which I believe is related) up to
the current tip of master, CLANGDWARF OVMF is broken. It builds and
starts but hangs early (more info below). I do not currently
On Thu, 7 Sept 2023 at 04:35, Mike Beaton wrote:
>
> The immediately preceding call, GetBestLanguage, plus the implementation of
> HiiGetString, which is called immediately afterwards, make it clear that
> BestLanguage is a null-terminated ASCII string, and not just a five byte
are currently used elsewhere in
the codebase for copying static strings).
Signed-off-by: Mike Beaton
---
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
b
elsewhere in the
codebase.
Signed-off-by: Mike Beaton
---
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
b/MdeModulePkg/Universal/HiiDatabaseDxe
On Wed, 6 Sept 2023 at 12:55, Mike Beaton wrote:
>
> On Wed, 6 Sept 2023 at 11:34, Mike Beaton wrote:
> >
> > AsciiStrLen was one byte too short (though with alignment up from an odd
> > size
> > would probably always have had the required space in practice).
On Wed, 6 Sept 2023 at 11:34, Mike Beaton wrote:
>
> AsciiStrLen was one byte too short (though with alignment up from an odd size
> would probably always have had the required space in practice). AsciiStrSize
> matches usage elsewhere in this file and in the codebase.
I was intend
AsciiStrLen was one byte too short (though with alignment up from an odd size
would probably always have had the required space in practice). AsciiStrSize
matches usage elsewhere in this file and in the codebase.
Signed-off-by: Mike Beaton
---
MdeModulePkg/Universal/HiiDatabaseDxe
-by: Mike Beaton
---
OvmfPkg/build.sh | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index b0334fb76e..279f0d099a 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -246,11 +246,11 @@ else
fi
#
-# Build the edk2 OvmfPkg
On Mon, 4 Sept 2023, 21:14 Rebecca Cran, wrote:
> Sorry I've not responded to your emails before now.
>
> I don't normally use the build.sh script, so I'm fine with reverting to
> the previous behavior.
>
Ah - thank you for the response - that'd be great, if the original people
who ack-d and
aviour. It
occurred to me that replying to the original post might make most
sense. I've raised what I think are the issues here:
https://bugzilla.tianocore.org/show_bug.cgi?id=4528 .
WIth many thanks in advance for your attention.
Mike Beaton
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive
PS Extra whitespace line was in original file, i.e. revert here is technically
correct, but assume v2 revert is preferable.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108049): https://edk2.groups.io/g/devel/message/108049
Mute This
Apologies, no Signed-off-by, I will send v2.
On Thu, 24 Aug 2023 at 06:52, Mike Beaton wrote:
>
> ---
> MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Univer
Signed-off-by: Mike Beaton
---
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
index
---
MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigKeywordHandler.c
index 96e05d4cf9..f67b7760f0 100644
---
This reverts commit 173a7a7daaad560cd69e1000faca1d2b91774c46.
Fixes https://bugzilla.tianocore.org/show_bug.cgi?id=4528
Signed-off-by: Mike Beaton
---
OvmfPkg/build.sh | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index
This includes an extraneous whitespace line at end of file, I will send a v2.
On Thu, 24 Aug 2023 at 05:45, Mike Beaton wrote:
>
> This reverts commit 173a7a7daaad560cd69e1000faca1d2b91774c46.
>
> Fixes https://bugzilla.tianocore.org/show_bug.cgi?id=4528
>
> Signed-o
This reverts commit 173a7a7daaad560cd69e1000faca1d2b91774c46.
Fixes https://bugzilla.tianocore.org/show_bug.cgi?id=4528
Signed-off-by: Mike Beaton
---
OvmfPkg/build.sh | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index
This is now in https://bugzilla.tianocore.org/show_bug.cgi?id=4528 as
it should have been in the first place!
On Sun, 13 Aug 2023 at 11:34, Mike Beaton wrote:
>
> Perhaps I can briefly clarify:
>
> "This now no longer works" was too brief - of course the listed command d
of the VM.
The advantage of using `build.sh -a ... -b ... qemu ...` to launch QEMU
without building (as was previously possible) is that it automatically
selects the correct qemu command and correct built OVMF BIOS binary to
match the related build command.
Mike
On Sun, 13 Aug 2023 at 11:13, Mike Beaton w
and then:
`./build.sh -a X64 -b RELEASE qemu {my-qemu-flags}` to run OVMF
This now no longer works, since the second command also attempts to rebuild
OVMF every time you run it.
I believe the previous behaviour was intended and more useful.
Mike Beaton
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all
64 matches
Mail list logo
